Skip to content

Preview Doctor config writes during dry-run#99255

Draft
giodl73-repo wants to merge 1 commit into
openclaw:mainfrom
giodl73-repo:doctor-config-write-dry-run
Draft

Preview Doctor config writes during dry-run#99255
giodl73-repo wants to merge 1 commit into
openclaw:mainfrom
giodl73-repo:doctor-config-write-dry-run

Conversation

@giodl73-repo

@giodl73-repo giodl73-repo commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a limited Doctor dry-run preview for the final config-write contribution
  • report the write envelope: target path, reason, changed top-level keys, backup/update backup expectations, and write options
  • skip replaceConfigFile during dry-run while preserving the legacy update handoff skip behavior

Public surface

  • No public SDK/plugin/config contract changes; SDK surface report is unchanged.
  • This does not expose a new CLI flag by itself. It prepares Doctor internals so a dry-run mode can report useful config-write intent without doing a full config diff.

PR surface

  • 3 files changed, +168/-14.

Validation

  • node scripts/run-vitest.mjs src/flows/doctor-health-contributions.test.ts (62 tests)
  • pnpm exec oxfmt --check src/commands/doctor.types.ts src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.ts
  • pnpm exec oxlint src/commands/doctor.types.ts src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.ts
  • pnpm tsgo:core
  • node scripts/plugin-sdk-surface-report.mjs --check
  • git diff --check

@giodl73-repo

giodl73-repo commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

@clawsweeper

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

@openclaw-barnacle openclaw-barnacle Bot added commands Command implementations size: S maintainer Maintainer-authored PR labels Jul 2, 2026
@clawsweeper

clawsweeper Bot commented Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 2, 2026, 6:50 PM ET / 22:50 UTC.

Summary
The PR adds Doctor dry-run plumbing that previews a pending config-file write instead of calling replaceConfigFile.

PR surface: Source +85, Tests +69. Total +154 across 3 files.

Reproducibility: not applicable. This PR adds internal dry-run preview behavior rather than reporting a reproducible bug. Source inspection confirms the changed path is the final Doctor config-write contribution.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: migration/backfill/repair: src/commands/doctor.types.ts, 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.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #79166
Summary: This PR is a candidate partial implementation for the canonical Doctor dry-run/diff request; broader open PRs remain candidates for the full public contract.

Members:

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until 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:

  • [P1] Add redacted terminal output or logs from an actual Doctor dry-run showing the config-write preview and that no config write occurred.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body lists tests and checks but does not include redacted terminal output, logs, or live output from an actual Doctor dry-run showing the config-write preview and no persisted write. 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 PR body lists tests and static checks, but not real after-fix Doctor dry-run output proving the preview appears and config is not persisted.
  • [P1] This is one slice of the broader public Doctor dry-run/diff contract tracked elsewhere, so maintainers need to decide whether to land it independently or fold it into the broader PR.

Maintainer options:

  1. Decide the mitigation before merge
    Keep the focused implementation open for maintainer review, then either land it as internal groundwork with real proof or fold the same final-write preview behavior into the broader Doctor dry-run/diff PR.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] The remaining action is maintainer review of a draft protected PR plus contributor real-behavior proof, not an automated code repair.

Security
Cleared: No supply-chain, dependency, secret-handling, or concrete security regression was found in this small Doctor config-write preview diff.

Review details

Best possible solution:

Keep the focused implementation open for maintainer review, then either land it as internal groundwork with real proof or fold the same final-write preview behavior into the broader Doctor dry-run/diff PR.

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

Not applicable; this PR adds internal dry-run preview behavior rather than reporting a reproducible bug. Source inspection confirms the changed path is the final Doctor config-write contribution.

Is this the best way to solve the issue?

Unclear for the full issue: this is a reasonable final-write preview slice, but the best complete solution is still a command-wide Doctor dry-run/diff contract that covers config, state, plugin, service, and reporting effects.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P3: The PR is a small Doctor preview plumbing improvement with limited immediate user impact because it does not expose a public flag by itself.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists tests and checks but does not include redacted terminal output, logs, or live output from an actual Doctor dry-run showing the config-write preview and no persisted write. 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 +85, Tests +69. Total +154 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 2 99 14 +85
Tests 1 69 0 +69
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 168 14 +154

What I checked:

  • AGENTS policy applied: The root repository policy was read fully; its protected-label, PR review, and config/Doctor compatibility guidance affected this review. (AGENTS.md:47, 8229f62ac3be)
  • PR is protected for maintainer handling: Live PR metadata shows this open draft PR has the maintainer label, so conservative cleanup must keep it open for explicit maintainer handling.
  • Patch adds final config-write dry-run preview: At PR head, runWriteConfigHealth builds write options, skips the legacy update handoff, and when ctx.options.dryRun is true logs a preview plus a would-write-config repair effect instead of persisting config. (src/flows/doctor-health-contributions.ts:1251, 3adfe2f2a1bb)
  • Current main writes config without this preview branch: On current main, the same contribution writes with replaceConfigFile whenever shouldWriteConfig is true, then logs update and backup paths; there is no dry-run preview branch in this function. (src/flows/doctor-health-contributions.ts:1223, 8229f62ac3be)
  • Regression coverage added: The PR adds focused tests for ordinary dry-run preview, update write options, and legacy update handoff skip behavior without calling replaceConfigFile. (src/flows/doctor-health-contributions.test.ts:2082, 3adfe2f2a1bb)
  • Canonical Doctor dry-run tracker remains open: The open canonical issue tracks the broader public Doctor dry-run/diff mode; this PR is a partial implementation slice rather than a complete merged fix.

Likely related people:

  • giodl73-repo: GitHub path history shows many recent merged commits on src/flows/doctor-health-contributions.ts and src/flows/doctor-repair-flow.ts, including contribution ordering and structured Doctor health slices that this PR builds on. (role: recent Doctor health contributor; confidence: high; commits: e56fd1dc0466, 9a5f2f61e76f, 056378efd582; files: src/flows/doctor-health-contributions.ts, src/flows/doctor-repair-flow.ts)
  • vincentkoc: History shows work on Doctor prompt option types and config-flow paths adjacent to the typed DoctorOptions and config-write behavior touched here. (role: Doctor config and option-path contributor; confidence: medium; commits: b73a18dd27f3, 5f42a93e4d49, c8ab37f6fe64; files: src/commands/doctor.types.ts, src/commands/doctor-config-flow.ts, src/flows/doctor-health-contributions.ts)
  • steipete: History shows the Doctor flow-contribution extraction plus adjacent Doctor docs and repair-flow maintenance around the same command path. (role: Doctor orchestration and adjacent docs contributor; confidence: medium; commits: 7d6d642cb825, 77d15841d781, c8665c66ba96; files: src/flows/doctor-health-contributions.ts, src/flows/doctor-health.ts, src/flows/doctor-repair-flow.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.

@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. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jul 2, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jul 3, 2026
Closed
7 tasks
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 maintainer Maintainer-authored PR P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. 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.

1 participant

@giodl73-repo