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. |
|
Codex review: needs real behavior proof before merge. Reviewed September 5, 2026, 4:35 PM ET / 20:35 UTC. ClawSweeper reviewWhat this changesThe onboarding wizard allows blank text submissions and preserves the current input and inline Gateway error when validation rejects an answer. Merge readiness⛔ Blocked before merge - 3 items remain This remains a useful, focused fix for behavior still present on main and in the latest release. No concrete introduced correctness defect was found, but the previous review's rejection-and-correction proof gap remains. Priority: P2 Review scores
Verification
How this fits togetherCompanion renders onboarding questions supplied by the Gateway and submits users' answers through wizard RPCs. Gateway responses determine whether the UI advances or retains the current question for correction. flowchart LR
A[Gateway question] --> B[Companion wizard]
B --> C[User text answer]
C --> D[Gateway validation]
D --> E[Accepted answer]
D --> F[Rejected answer]
E --> G[Transcript and next question]
F --> B
Before merge
Agent review detailsSecurityNone. Review metricsNone. 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 Gateway-owned text validation, with optional blanks accepted and rejected answers recoverable inline without resetting input or advancing the transcript. Do we have a high-confidence way to reproduce the issue? Yes, source establishes that leaving Model alias empty disables Continue and fails the submission gate on current main; this review did not execute a failing native run. Is this the best way to solve the issue? Yes, delegating validation to the existing Gateway flow is narrower and more maintainable than guessing requiredness from labels; the added rejection path still needs real runtime evidence. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 305bb4ef5334. LabelsLabel changes:
Label 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 (3 earlier review cycles) |
Native Windows proof added —
|
|
@clawsweeper re-review Exact-head native Windows proof is now publicly linked in the PR body: blank Model alias (optional) with Continue enabled, followed by the next wizard step after submitting the blank value. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
This pull request has been automatically marked as stale due to inactivity. |
Closes #1331
What Problem This Solves
Fixes an issue where users configuring an OpenAI-compatible provider in Companion could not continue past Model alias (optional) without entering an alias.
It also resolves a related wizard-client problem where Gateway validation errors returned for a text answer could be cleared immediately, append a rejected answer to the transcript, and count the same validation retry as another step visit.
Why This Change Was Made
The Gateway wizard protocol does not expose whether an individual text step is required, while the Gateway already owns each text prompt's authoritative validator. Companion now submits the current text value, including an empty string, and handles a successful non-terminal response containing an error plus the same step as an inline validation rejection.
This keeps optional fields usable without guessing requiredness from localized labels. Required fields remain protected by the Gateway validator.
User Impact
Users can leave optional text fields such as Model alias (optional) empty and continue onboarding. If a required or invalid value is rejected by the Gateway, Companion keeps the current input visible, shows the validation message, and does not add the rejected answer to the transcript.
Evidence
git diff --checkpassed at commitc6b73ad1.20d0daa063c0cc5eaee98043b673d413be5945f4:WizardSession.answerruns the prompt validator and keeps the current step pending when validation returns an error;wizard.nextreturns a successful non-terminal payload containing thaterrorplussession.next(), which is the same current step.Change Type
Scope
winnodeRequired proof pools
windows-winui-interactive: the onboarding wizard's Continue state and inline Gateway validation-error behavior changed.Validation
Local required validation was attempted but could not start on the macOS authoring host:
./build.ps1- blocked, PowerShell and the Windows build environment are unavailable.dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore- blocked, .NET 10 SDK is unavailable.dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore- blocked, .NET 10 SDK is unavailable.WizardSelectionTests- blocked, .NET 10 SDK is unavailable.git diff --check- passed.No automated test is reported as passing without a non-zero test count. GitHub CI and Windows proof remain required before this draft is ready for review.
Real Behavior Proof
run-app-local.ps1 -Dev -Isolated -AllowNonMain).c6b73ad1237e92ee7ff730ccbea70ee66ecd4991(verified in the Windows checkout immediately after the run).{"Title":"Enter value","Message":"Model alias (optional)","ButtonEnabled":true,"ValueLength":0}{"Action":"Invoke PrimaryButton","Result":"INVOKED_BLANK_ALIAS"} {"Title":"Enter value","Message":"Gateway port","ButtonEnabled":true}Focused, sanitized screenshots
Blank optional alias; Continue enabled at exact PR code commit:
After submission: the checked, value-less transcript entry records the blank answer, and the wizard is now on Gateway port:
The image URLs and proof notes were verified publicly resolvable. Captures exclude provider endpoint, endpoint ID, model ID, API-key state, gateway records, and unrelated desktop content.
Pre-fix evidence: the reporter's original screenshot directly shows the same blank Model alias (optional) field with Continue disabled, but it remains private and is not republished here. Current-main source at
305bb4ef5334ded9f8541aca19c66ba2c4b08754independently reproduces that client gate: empty text disables Continue and is rejected beforewizard.next.Not verified / blocked: GitHub fork CI remains
action_requiredpending upstream approval.Security Impact
Yes/No): NoYes/No): NoYes/No): NoYes/No): NoYes/No): NoYes, explain the risk and mitigation: N/ACompatibility and Migration
Yes/No): YesYes/No): NoYes/No): NoReview Conversations