Skip to content

feat(wizard): deliver progress over the RPC wizard protocol#94664

Closed
bkudiess wants to merge 1 commit into
openclaw:mainfrom
bkudiess:feat/wizard-rpc-progress
Closed

feat(wizard): deliver progress over the RPC wizard protocol#94664
bkudiess wants to merge 1 commit into
openclaw:mainfrom
bkudiess:feat/wizard-rpc-progress

Conversation

@bkudiess

Copy link
Copy Markdown
Contributor

Summary

Part of #94661 (wizard RPC parity for non-terminal clients).

The setup wizard runs locally (terminal clack prompter) and over an RPC protocol so non-terminal clients (the Windows companion, remote nodes) can drive onboarding. The RPC WizardSessionPrompter.progress() was a no-op (src/wizard/session.ts), so every spin.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 progress steps over the existing wizard protocol. The progress step type and optional message are already in the wizard step schema, so no protocol schema change is needed.

How it works

  • progress() / update() / stop(msg) emit a progress step (type "progress", executor: "client").
  • Latest-wins: a single pending-progress slot, not an unbounded queue — a client only needs the current label.
  • Progress is drained ahead of the pending prompt, and a long-polling wizard.next is woken mid-operation so status is live.
  • Client contract: progress steps are non-interactive; the client re-polls wizard.next without an answer to continue. Progress steps are not answerable.
  • Local CLI is unchanged (it uses the clack prompter, not WizardSession).

Change type

  • Bug fix (RPC clients receive progress that was previously dropped)

Scope

  • Gateway / orchestration
  • API / contracts (additive; no schema change)
  • UI / DX

Real behavior proof

Behavior addressed: progress emitted during the RPC wizard is now delivered to clients as progress steps 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 waiting next() mid-operation, are not answerable, and are cleared on cancel. All pass; tsgo:core / tsgo:core:test exit 0; lint and format-check pass.
Observed result after fix: wizard.next returns { 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-repo pnpm check / pnpm test.

Compatibility

  • Backward compatible? Yes — additive. Clients that ignore progress steps are unaffected; no schema change.
  • Config/env changes? No. Migration needed? No.

Security impact

No new permissions, secrets handling, network calls, or execution surface. Progress messages are non-secret status labels.

AI-assisted: Yes.

@openclaw-barnacle openclaw-barnacle Bot added size: S triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels Jun 18, 2026
@clawsweeper

clawsweeper Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 25, 2026, 2:39 PM ET / 18:39 UTC.

Summary
The PR changes WizardSessionPrompter.progress() to emit latest-wins progress steps over wizard.next and adds WizardSession tests for progress delivery, cancellation, concurrency, and answer rejection.

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.

  • Progress-answering RPC consumers: 2 bundled clients plus 1 gateway E2E loop. Current in-repo consumers submit answers for wizard steps that can include progress, so compatibility needs review before merge.

Stored data model
Persistent data-model change detected: serialized state: src/wizard/session.test.ts, serialized state: src/wizard/session.ts, unknown-data-model-change: src/wizard/session.test.ts, unknown-data-model-change: src/wizard/session.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #94661
Summary: This PR is a candidate fix for the progress sub-gap of the broader wizard RPC parity issue; sibling OAuth and device-pairing work is related but distinct.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Preserve or gate current RPC wizard clients that still answer progress steps.
  • [P1] Add redacted real RPC wizard client proof showing visible progress and advancement to the next prompt.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body and follow-up comment report tests, type checks, lint, and format checks only; it still needs redacted real RPC wizard client output, logs, screenshot, recording, or linked artifact showing progress delivery and next-prompt advancement. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Proof path suggestion
A real RPC wizard client recording would materially show whether progress is visible and whether the flow advances without answering progress. Mantis is currently scoped to Telegram, Discord, and web UI chat proof, so it is not the right proof path for this surface.
Use maintainer screenshot/manual proof, browser or Playwright proof, Crabbox where appropriate, or normal local artifact proof instead.

Risk before merge

  • [P1] Existing bundled RPC wizard clients can hit wizard: no pending step or stall when they answer a newly emitted progress step.
  • [P1] The PR has focused unit coverage but no redacted real RPC wizard client proof showing progress rendering and advancement to the next prompt.

Maintainer options:

  1. Preserve Progress-Answering Clients (recommended)
    Make current RPC wizard clients advance safely when they receive progress, either by tolerating progress answers server-side or updating bundled clients before broad emission.
  2. Accept A Client Contract Break
    Maintainers may intentionally require every client to re-poll without answering progress, but that should be an explicit compatibility decision with rollout proof.
  3. Pause For Protocol Direction
    Pause this branch if maintainers want the broader wizard RPC parity issue to settle progress-step semantics before core emits progress frames.

Next step before merge

  • [P1] Human review is needed because the remaining blockers are client-visible protocol compatibility and contributor-side real behavior proof, not a safe standalone repair job.

Security
Cleared: No concrete security or supply-chain concern found; the diff changes in-memory wizard session flow and tests without new dependencies, permissions, scripts, credentials, downloads, or secret handling.

Review findings

  • [P1] Preserve clients that still answer progress steps — src/wizard/session.ts:162
Review details

Best 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. WizardSession is the right owner boundary, but the best fix must preserve or explicitly gate current progress-answering RPC clients and include real-client proof.

Full review comments:

  • [P1] Preserve clients that still answer progress steps — src/wizard/session.ts:162
    This starts returning progress steps to every RPC wizard client, but current bundled clients still submit an answer for progress steps. Because progress ids are never added to answerDeferred, those clients can get wizard: no pending step or INVALID_REQUEST instead of advancing; tolerate progress answers, update bundled clients, or gate emission before rollout.
    Confidence: 0.93

Overall correctness: patch is incorrect
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 66e2fcc6f83e.

Label changes

Label justifications:

  • P1: The PR targets visible non-terminal onboarding progress but can break existing bundled RPC wizard clients during setup.
  • merge-risk: 🚨 compatibility: The patch emits unanswerable progress steps while current bundled wizard clients still submit answers for progress-capable steps.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body and follow-up comment report tests, type checks, lint, and format checks only; it still needs redacted real RPC wizard client output, logs, screenshot, recording, or linked artifact showing progress delivery and next-prompt advancement. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +62, Tests +129. Total +191 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 82 20 +62
Tests 1 129 0 +129
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 211 20 +191

What I checked:

  • AGENTS policy applied: Root AGENTS.md and the gateway scoped guides were read; their compatibility-sensitive gateway protocol and proof-gate guidance directly affected this review. (AGENTS.md:21, 66e2fcc6f83e)
  • Current main drops RPC wizard progress: On current main, WizardSessionPrompter.progress() still returns no-op update and stop callbacks, so the reported progress-delivery gap is real. (src/wizard/session.ts:158, 66e2fcc6f83e)
  • Protocol already admits progress steps: WizardStepSchema already includes the progress literal, so this PR changes delivery semantics rather than adding a new schema literal. (packages/gateway-protocol/src/schema/wizard.ts:68, 66e2fcc6f83e)
  • PR emits unanswerable progress frames: At PR head, progress() emits a progress step and the new tests intentionally assert that answering a progress step throws wizard: no pending step. (src/wizard/session.ts:162, fb150b456f28)
  • Gateway rejects answers for non-pending ids: wizard.next forwards any supplied answer to session.answer() and returns INVALID_REQUEST when that call throws, making answered progress ids client-visible failures. (src/gateway/server-methods/wizard.ts:77, 66e2fcc6f83e)
  • Bundled macOS app answers progress: The current macOS onboarding model always sends an answer payload for submitted steps, and the step view treats note and progress as submit-with-nil steps. (apps/macos/Sources/OpenClaw/OnboardingWizard.swift:103, 66e2fcc6f83e)

Likely related people:

  • vincentkoc: Recent GitHub history shows direct work on src/wizard/prompts.ts and src/wizard/session.ts in the prompt-session type refactor near this PR's changed surface. (role: recent wizard session contributor; confidence: medium; commits: f3ae5252115e; files: src/wizard/prompts.ts, src/wizard/session.ts)
  • steipete: History shows the original unified onboarding/config-schema work that introduced the wizard session path plus repeated follow-up work on gateway wizard/protocol and macOS client surfaces. (role: wizard and gateway protocol feature contributor; confidence: medium; commits: 53baba71faa3, 3f815fad1293; files: src/wizard/session.ts, src/wizard/session.test.ts, apps/macos/Sources/OpenClawMacCLI/WizardCommand.swift)
  • BunsDev: Recent history aligned the macOS wizard CLI with generated protocol models, and that CLI is one of the current progress-answering RPC clients affected by this PR. (role: recent macOS wizard CLI contributor; confidence: low; commits: dafbdb6f20fd; files: apps/macos/Sources/OpenClawMacCLI/WizardCommand.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 18, 2026
@bkudiess
bkudiess force-pushed the feat/wizard-rpc-progress branch from ce88885 to 37a2b11 Compare June 18, 2026 18:39
@bkudiess

Copy link
Copy Markdown
Contributor Author

Hardened the progress delivery after an adversarial review pass.

Found and fixed a concurrency bug: when two wizard.next calls were in flight for the same session, they shared one internal step waiter. A progress emit resolves that waiter with null while the session is still running, so one caller consumed the progress and the other could return a spurious { done: true, status: "running" }.

Fix: next() now loops and never decides completion off a raw wake — a null resolution is re-classified via the pending-progress / current-step / terminal / status checks, so a progress wake consumed by a concurrent caller just makes the other caller re-wait. Added a regression test (concurrent next() callers never get a spurious done while running) that fails on the old code and passes on the fix.

Re-validated locally on latest main: src/wizard/session.test.ts 11/11; tsgo:core, tsgo:core:test, oxlint, and oxfmt all clean.

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]>
@bkudiess
bkudiess force-pushed the feat/wizard-rpc-progress branch from 37a2b11 to fb150b4 Compare June 18, 2026 20:31
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

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.

@steipete steipete closed this Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants