Repository navigation
refactor(doctor): simplify preview policy warnings - #140124
Merged
Merged
Conversation
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
steipete
marked this pull request as ready for review
September 6, 2026 13:08
Contributor
|
Codex review: blocked before merge. Reviewed September 6, 2026, 9:46 AM ET / 13:46 UTC. ClawSweeper reviewWhat this changesSimplifies Doctor’s configuration-warning helpers and removes two unnecessary type assertions while preserving warning text, ordering, and policy checks. Merge readiness⛔ Blocked before merge - 1 item remains Keep this maintainer-requested cleanup open with no actionable findings. The previous migration-proof blocker was inapplicable: the introduced changes do not alter persisted data or upgrade behavior. Priority: P3 Review scores
Verification
How this fits togetherDoctor examines OpenClaw configuration and presents diagnostic notes to operators. This change simplifies how its preview combines agent, provider, and reply policies into warnings. flowchart TD
A[OpenClaw configuration] --> B[Doctor preview]
B --> C[Agent and channel routes]
B --> D[Provider and tool policies]
C --> E[Warning collectors]
D --> E
E --> F[Operator diagnostic notes]
Before merge
Agent review detailsSecurityNone. PR surfaceSource -60, Other -1. Total -61 across 2 files. View PR surface stats
Review metricsNone. Stored data modelPersistent data-model change detected: Technical reviewBest possible solution: Retain the simpler private helpers and existing warning contract without adding migration machinery or duplicate tests. Do we have a high-confidence way to reproduce the issue? Not applicable: this refactor claims behavioral equivalence rather than a bug fix; source inspection and existing warning tests support that claim. Is this the best way to solve the issue? Yes. Reusing the existing resolver and loader, preserving inferred roster types, and removing an unused branch is a narrow simplification of the current implementation. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against b8ef27d69475. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
steipete
force-pushed
the
steipete/cleanup-doctor-preview-relays
branch
from
September 6, 2026 13:40
029f7d8 to
b346bb6
Compare
Contributor
Author
|
Merged via squash.
|
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Sep 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related: #135868
What Problem This Solves
Doctor preview duplicated message-tool availability requests and carried private wrappers and provenance fields that no caller needed. This is a maintainer-requested cleanup companion to the #135868 split campaign; it extracts no recovery feature content.
Why This Change Was Made
Use the existing provider-policy and lazy-loader owners directly, build each availability request once, and remove the source-reply helper's unreachable no-grant option. Both callers always enabled that grant. Keep the filtered agent list's canonical inferred type, eliminating the two duplicated assertions and their exact baseline entry.
User Impact
No user-visible behavior changes. Warning text and ordering, explicit provider/global/agent restrictions, direct/group reply selection, malformed-entry filtering, and lazy channel loading remain intact.
Evidence
v2026.9.2and the inspected main base have the identical original owner blob. Pickaxe history traces these helpers toe357203be57; this removes private plumbing while preserving the shipped warning contract.Initial exact-head CI run 34035193405 failed in the existing Codex containment fixture,
transport.process.test.ts:385(the reparented case reports uncertain cleanup). Its test and runtime owners are unchanged on current main. The lead-requested second attempt passed on the same reviewed head; no test or runtime change was made. The final fresh-main rebase preserves all recorded proof inputs and the lockfile, and final-head CI passed in run 34036825802. Native prepare and merge completed successfully; merged as d4f67f2.ClawSweeper disposition (reviewed
029f7d840cca): no code or security findings. Its requested data-model compatibility proof is inapplicable: the changed owner only reads configuration and produces transient diagnostic strings; the removed provenance field was private temporary data and never persisted. No schema, store, migration, serialization, config key, or write path changed. The other file is a tooling assertion-count ratchet, not an OpenClaw data store. Stable-tag source comparison, frozen-input comparisons, unchanged warning output, and existing Doctor tests cover the actual compatibility contract. No additional migration test or data-model change is warranted.Follow-up diagnosis on the unchanged reviewed head: the targeted Codex case passed locally on macOS and in all ten Linux Testbox trials (run 34035880137; one-shot lease stopped). These trials did not establish the initial failure’s root cause. The subsequent lead-requested CI attempt passed; no assertion was weakened and no rerun is claimed as a source repair.