← Back to PRs

#21240: fix: GH#20607 prevent doctor from dropping custom config sections

by theognis1002 open 2026-02-19 20:25 View on GitHub →
commands size: S
## Summary - Problem: `openclaw doctor` can silently drop the `memory` config section (and potentially other top-level sections), causing OpenClaw to fall back from QMD to builtin vector memory without warning. - Why it matters: Users lose cross-session recall with no indication of what happened. - What changed: (1) `stripUnknownConfigKeys` no longer mutates `candidate` when `shouldRepair=false`, (2) `writeConfigFile` guards against merge-patch spuriously deleting top-level sections, (3) `loadAndMaybeMigrateDoctorConfig` restores any recognized schema keys dropped during processing. - What did NOT change (scope boundary): No changes to the schema, memory subsystem, or config read path. ## Change Type (select all) - [x] Bug fix ## Scope (select all touched areas) - [x] Memory / storage ## Linked Issue/PR - Closes #20607 ## User-visible / Behavior Changes - `openclaw doctor` no longer silently drops recognized config sections (e.g., `memory.backend: "qmd"`) during repair or non-repair flows. - A note is logged if a dropped section is restored. ## Security Impact (required) - New permissions/capabilities? No - Secrets/tokens handling changed? No - New/changed network calls? No - Command/tool execution surface changed? No - Data access scope changed? No ## Repro + Verification ### Steps 1. Configure `memory.backend: "qmd"` with full QMD config 2. Add a pending change trigger (e.g., a legacy dm.policy alias) 3. Run `openclaw doctor` 4. Check if `memory` section is preserved ### Expected - Memory section fully preserved in output config ### Actual (before fix) - Memory section silently dropped ## Evidence - [x] Failing test/log before + passing after - Two new e2e regression tests added in `doctor-config-flow.e2e.test.ts` ## Human Verification (required) - Verified scenarios: Both repair and non-repair paths preserve memory config - Edge cases checked: Unknown keys alongside recognized memory section - What you did **not** verify: Full `pnpm test` suite (targeted tests pass) ## Compatibility / Migration - Backward compatible? Yes - Config/env changes? No - Migration needed? No ## Failure Recovery (if this breaks) - How to disable/revert this change quickly: Revert this commit - Files/config to restore: `src/config/io.ts`, `src/commands/doctor-config-flow.ts` - Known bad symptoms reviewers should watch for: Config sections being duplicated or stale values being restored when they should have been intentionally removed ## Risks and Mitigations - Risk: The section-drop guard could restore stale config values if a section was intentionally removed by a future doctor step. - Mitigation: Guard only restores keys that existed in the original `baseCfg` and are recognized by the schema. Intentional removals by doctor steps should explicitly handle this. AI-assisted: Yes (Claude). Lightly tested (targeted e2e tests pass). <!-- greptile_comment --> <h3>Greptile Summary</h3> Fixed critical bug where `openclaw doctor` silently dropped valid config sections like `memory` when unknown keys were present. The fix prevents mutation of the working config in non-repair mode and adds two safety guards to restore any inadvertently dropped schema sections. **Changes:** - `stripUnknownConfigKeys` in doctor-config-flow.ts:897-909 now only mutates `candidate` when `shouldRepair=true`, preserving the original config in non-repair mode - Added restoration logic in loadAndMaybeMigrateDoctorConfig:926-936 that checks for and restores any schema-valid sections that were dropped during processing - Added merge-patch guard in writeConfigFile:830-844 to prevent spurious deletion of top-level sections when config defaults differ between reads **Test coverage:** - Two new e2e regression tests verify memory section preservation in both repair and non-repair modes - Existing tests confirm unknown keys are still properly removed when expected <h3>Confidence Score: 4/5</h3> - This PR is safe to merge with minimal risk - The fix addresses a real bug with a well-understood root cause and includes targeted e2e tests. The changes are defensive (adding safety guards) rather than risky refactoring. Score reflects minor concern about restoration logic potentially masking intentional deletions in future doctor steps, but risk is mitigated by schema-key checks and clear scoping - No files require special attention <sub>Last reviewed commit: 00191ae</sub> <!-- greptile_other_comments_section --> <sub>(4/5) You can add custom instructions or style guidelines for the agent [here](https://app.greptile.com/review/github)!</sub> <!-- /greptile_comment -->

Most Similar PRs