feat(wizard): deliver progress over the RPC wizard protocol#94664
feat(wizard): deliver progress over the RPC wizard protocol#94664bkudiess wants to merge 1 commit into
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 25, 2026, 2:39 PM ET / 18:39 UTC. Summary PR surface: Source +62, Tests +129. Total +191 across 2 files. Reproducibility: yes. Source inspection on current main shows RPC wizard progress is dropped, and PR head emits progress steps that current bundled clients still answer. Review metrics: 1 noteworthy metric.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Proof path suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Emit progress in-band while preserving current clients that answer progress steps, document the non-interactive semantics, and require redacted real RPC-client proof before merge. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection on current main shows RPC wizard progress is dropped, and PR head emits progress steps that current bundled clients still answer. Is this the best way to solve the issue? No. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 66e2fcc6f83e. Label changesLabel justifications:
Evidence reviewedPR surface: Source +62, Tests +129. Total +191 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
ce88885 to
37a2b11
Compare
|
Hardened the progress delivery after an adversarial review pass. Found and fixed a concurrency bug: when two Fix: Re-validated locally on latest |
WizardSessionPrompter.progress() was a no-op, so every spin.update/stop during installs, plugin/channel downloads, and auth waits was dropped for non-terminal clients (e.g. the Windows companion), which appeared frozen during the slowest parts of onboarding. Emit progress as non-interactive 'progress' steps (already in the wizard step schema). Latest-wins single slot, drained ahead of the pending prompt; next() loops so a progress wake never decides completion, which keeps concurrent next() callers from surfacing a spurious done while running. Clients re-poll without an answer to continue; progress steps are not answerable. Refs openclaw#94661. Co-authored-by: Copilot <[email protected]>
37a2b11 to
fb150b4
Compare
|
Thanks for the contribution. I’m closing this stalled PR because the real RPC-wizard client proof requested on June 18 is still missing, the compatibility blocker remains, and the head has had no subsequent commit or check activity. The canonical issue remains open; a fresh focused PR with current-main compatibility and end-to-end RPC proof is welcome. |
Summary
Part of #94661 (wizard RPC parity for non-terminal clients).
The setup wizard runs locally (terminal
clackprompter) and over an RPC protocol so non-terminal clients (the Windows companion, remote nodes) can drive onboarding. The RPCWizardSessionPrompter.progress()was a no-op (src/wizard/session.ts), so everyspin.update(...)/stop(...)during installs, plugin/channel downloads, and OAuth/device-code waits was silently discarded for RPC clients — the companion looked frozen during the slowest parts of onboarding.This delivers progress as non-interactive
progresssteps over the existing wizard protocol. Theprogressstep type and optionalmessageare already in the wizard step schema, so no protocol schema change is needed.How it works
progress()/update()/stop(msg)emit aprogressstep (type"progress",executor: "client").wizard.nextis woken mid-operation so status is live.wizard.nextwithout an answer to continue. Progress steps are not answerable.WizardSession).Change type
Scope
Real behavior proof
Behavior addressed: progress emitted during the RPC wizard is now delivered to clients as
progresssteps instead of being dropped, so non-terminal clients show live status during slow onboarding steps.Real environment tested: local OpenClaw worktree on latest
main(Windows).Exact steps or command run after this patch:
node scripts/run-vitest.mjs run src/wizard/session.test.ts(10 tests, incl. 4 new);pnpm tsgo:core;pnpm tsgo:core:test;node scripts/run-oxlint.mjs src/wizard/session.ts src/wizard/session.test.ts;pnpm format:check.Evidence after fix: New tests assert progress steps are emitted (
type: "progress"), delivered before the next prompt, collapse latest-wins, wake a waitingnext()mid-operation, are not answerable, and are cleared on cancel. All pass;tsgo:core/tsgo:core:testexit 0; lint and format-check pass.Observed result after fix:
wizard.nextreturns{ done:false, step:{ type:"progress", message } }for in-flight progress; re-polling without an answer advances to the next prompt.What was not tested: Windows companion rendering of progress steps (lands separately in
openclaw-windows-node), and full-repopnpm check/pnpm test.Compatibility
progresssteps are unaffected; no schema change.Security impact
No new permissions, secrets handling, network calls, or execution surface. Progress messages are non-secret status labels.
AI-assisted: Yes.