fix: reject mixed candidate preflight failures - #165
Merged
steipete merged 2 commits intoSep 16, 2026
Merged
Conversation
|
🦞👀 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. |
clawsweeper
Bot
added
P2
|
Codex review: needs real behavior proof before merge. Reviewed September 16, 2026, 12:10 AM ET / 04:10 UTC (Revision 6). ClawSweeper reviewWhat this changesTightens OCM’s upgrade preflight parser to reject mixed or malformed failures, with regression tests and documentation preserving the sole-unsupported compatibility exception. Merge readiness⛔ Blocked before merge - 3 items remain The fix remains necessary: current main and v0.2.47 still accept mixed candidate failures. The earlier skipped-count defect is resolved and no blocking code finding remains, but the supplied evidence still uses synthetic candidates and does not establish real upgrade compatibility. Priority: P2 Review scores
Verification
How this fits togetherOCM runs the candidate OpenClaw runtime’s Doctor check after configuration repair and before finalization. The command’s result determines whether OCM publishes the new runtime binding or follows its existing failure and recovery path. flowchart TD
A[Requested runtime upgrade] --> B[Repair target configuration]
B --> C[Run candidate Doctor]
C --> D{Successful or solely unsupported?}
D -->|Yes| E[Finalize candidate]
E --> F[Publish runtime binding]
D -->|No| G[Fail upgrade and recover]
Before merge
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep complete-result validation at the existing preflight boundary, with upgrade evidence confirming genuine legacy candidates still succeed and rejected candidates preserve recovery behavior. Do we have a high-confidence way to reproduce the issue? Yes, from source: a nonzero candidate response containing an unsupported marker plus a genuine error satisfies main’s classifier and returns success. This review did not execute the path. Is this the best way to solve the issue? Yes, the existing classifier is the narrowest repair boundary and the patch preserves configuration-repair ordering; historical diagnostic compatibility still needs runtime evidence. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against b1557f6aa7df. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (5 earlier review cycles)
|
Require a complete sole-unsupported result before bypassing candidate validation. Reject missing or contradictory status fields, malformed JSON, extra findings and mixed diagnostics. Preserve registry-dependent skipped counts and narrow legacy command/option compatibility; retain the previous binding without finalizing rejected candidates. Co-authored-by: goutamadwant <workwithgoutam@gmail.com>
steipete
force-pushed
the
fix/candidate-preflight-mixed-failures
branch
from
September 16, 2026 04:04
8c01be1 to
2c5c34b
Compare
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
2 participants
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.
Closes #128.
What Problem This Solves
Candidate preflight could report an unsupported check alongside a real validation error, yet OCM would finalize the candidate and switch the environment to it.
User Impact
Mixed, malformed, incomplete, and contradictory failures now stop the upgrade before finalization and binding publication. Candidates whose sole failure is the unsupported check remain compatible, including registry-dependent skipped counts and legacy unsupported command/option diagnostics.
Why This Change Was Made
Require a complete failed JSON result with zero executed checks, a numeric skipped count, and exactly one matching error finding; reject other diagnostics. The text fallback accepts only an unambiguous unsupported command or option plus usage guidance. This retains @goutamadwant's fix and regression coverage, closes the missing-status-field gap found during independent review, and documents the compatibility boundary.
Evidence
On unchanged production code, the new built-CLI regression returned
outcome=switchedafter a candidate reported both unsupported selection and invalid configuration. After the fix it fails before finalization and retainsold-local. Unit coverage includes complete sole-unsupported results, registry skipped counts, mixed findings, missing fields, contradictory status, malformed JSON, and appended fatal text. The output contract was checked against OpenClaw's lint formatter.Behavior proof uses the built OCM CLI and isolated synthetic candidate/service fixtures. No production gateway or credentials were used. Formatting, all-target checking, and the candidate unit matrix pass. After updating two older test stubs to emit the complete upstream JSON shape, 74 of 75 native upgrade cases passed together; the one preparation case returned an empty test-HTTP response and passed in isolation. The isolated Linux full run passed all other test targets and identified those two incomplete stubs before correction. Final exact-head cross-platform CI remains the merge gate.
Rebased on merged #149 at
88d5145; the shared fixture repairs are already on main. The only rebase conflict was the adjacent changelog entry, and both entries are retained. The final patch changes only candidate validation, its tests, usage documentation, and the changelog. Independent review is clean at P0–P2.All nine cross-platform CI jobs passed on final head
c8f59eb9b49d5f8f942ea427f186e4bd51366edd, including full macOS and Linux test suites and installation smoke checks: https://github.com/openclaw/ocm/actions/runs/35054314790.