Skip to content

fix(onboarding): restore post-wizard handoff and chat connection; properly bootstrap hatching prompt - #312

Merged
shanselman merged 17 commits into
openclaw:masterfrom
indierawk2k2:fix/bootstrap-injector-properly
May 12, 2026
Merged

fix(onboarding): restore post-wizard handoff and chat connection; properly bootstrap hatching prompt#312
shanselman merged 17 commits into
openclaw:masterfrom
indierawk2k2:fix/bootstrap-injector-properly

Conversation

@indierawk2k2

@indierawk2k2 indierawk2k2 commented May 11, 2026

Copy link
Copy Markdown
Contributor

Fixes regressions introduced by #307 and properly addresses the hatching-prompt bug it tried to fix.

Hand-off chain

After onboarding finishes on the Ready page, OnboardingWindow.OnWizardComplete() checks that setup is no longer required, then defers ShowHubChatAfterWizardClose() onto the dispatcher. That opens App.ShowHub("chat"), routes the HubWindow to the Chat tab, initializes ChatPage, connects the WebView to the gateway with resolved operator credentials, and runs BootstrapMessageInjector after successful navigation.

What changed

  • Restored the post-wizard Hub chat-tab launch path and made completion idempotent for Finish/X close paths.
  • Fixed setup-state detection for local node mode by requiring the stored node-role token before skipping setup.
  • Reused the interactive gateway credential resolver in ChatPage so the Chat tab connects with the same operator credentials as the rest of the tray.
  • Reworked BootstrapMessageInjector into a defensive, testable one-shot service that broad-matches composer primitives across document/open shadow roots, verifies the prompt rendered, and never lets injector failures bubble into the wizard/chat handoff.
  • Added/updated tests for bootstrap gate behavior, safe JS encoding, broad composer discovery, startup setup state, manager-node suppression, concurrent injection, sent-vs-rendered status semantics, chat-readiness gating, and accepted-send proof.

Follow-up commits address double-init race

These follow-up commits implement Aaron's 2026-05-11 RCA fixes for bugs A/B/C:

  • (A) PR Connection architecture refactoring: manager-owned lifecycle, setup integration, diagnostics #304 introduced a manager-owned NodeConnector startup path after operator handshake. That path is still preserved for remote/no-local-setup gateways, but it is now suppressed for an active local gateway when the canonical local NodeService already owns a stored Windows-node identity under IdentityDataPath.
  • (B) BootstrapMessageInjector.InjectAsync(...) now has an in-process in-flight guard and re-checks HasInjectedFirstRunBootstrap after the initial delay, immediately before JavaScript execution, so two WebView navigations cannot both pass the pre-delay gate and execute.
  • (C) Injector status handling now distinguishes sent from rendered; only sent consumes HasInjectedFirstRunBootstrap, while rendered logs that the prompt reached the composer without confirmed submit and leaves the gate open for retry.

Round 3 ΓÇö chat readiness gate + accepted-send proof

  • (D) ChatPage now opens the Chat tab immediately but defers WebView.CoreWebView2.Navigate(...) until the active GatewayConnectionManager operator state reaches the handshake-succeeded/hello-ok boundary and the tokenized chat HTTP URL returns a successful response. While waiting, it shows a lightweight ╬ô├ç┬úWaiting for chat to start╬ô├ç┬¬╬ô├ç┬Ñ state; if readiness times out, it logs the failure and shows an inline Retry button instead of exposing a WebView 404.
  • (E) BootstrapMessageInjector now treats submit/click as only an attempt. It returns proven sent only after bounded polling observes accepted-send proof: the composer cleared or the hatching text appears in a user-message-like transcript element. If proof is not observed, it returns unconfirmed and leaves HasInjectedFirstRunBootstrap open for retry.

What we deliberately did NOT change

⚠️ REQUIRED MANUAL VERIFICATION BEFORE MERGE

Mike must walk the real WSL wizard end-to-end before merge and verify all six checks:

  1. (A) Completing the post-wizard Ready/Finish flow auto-launches the tray Hub on the Chat tab.
  2. (B) The Chat tab connects to the gateway using the paired operator credentials and does not show the pairing/bootstrap-token error.
  3. (C) The first-run hatching prompt from BOOTSTRAP.md submits, not just types; an assistant reply is visible in the chat transcript.
  4. (D) No second Windows-node pairing notification appears after Chat opens.
  5. (E) wsl -d OpenClawGateway -- cat ~/.openclaw/devices/paired.json shows exactly one Windows-node entry, not two.
  6. (F) The WebView never shows a 404; it shows a brief waiting state and then the chat default state directly.

I reset WSL validation state, launched with OPENCLAW_VISUAL_TEST=1, confirmed the app starts without startup exceptions, captured visual-test-output\verify\openclaw-setup-smoke.png, and also captured an isolated Chat smoke screenshot at visual-test-output\verify-chat\chat-waiting-desktop.png. I could not fully drive the wizard end-to-end without Mike completing the interactive WSL flow, so A/B/C/D/E/F remain required.

Validation

  • Initial final ./build.ps1 retry was briefly blocked by a transient file lock on OpenClaw.Shared.deps.json; rerunning after the lock cleared passed.
  • ./build.ps1 passed: Shared, Cli, WinNodeCli, and WinUI all built.
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore passed with OPENCLAW_REPO_ROOT set: 1465 total, 1443 passed, 22 skipped.
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore passed with OPENCLAW_REPO_ROOT set: 924 total, 924 passed.

Expertise

See .squad/agents/aaron/expertise/onboarding-chat-bootstrap.md for PR archeology, the hidden defensive-injector contract, selector/timing rationale, the dual-startup-path hazard, the injector race-window pattern, the chat-readiness gate, and the accepted-send-proof pattern.

Round 4 ΓÇö Option Z: gateway-side bootstrap (no DOM injection)

  • Implements Mike's permanent no-DOM-injection directive for chat automation: the hatching kickoff now goes through the authenticated gateway chat.send WebSocket API instead of clicking/filling WebView DOM.
  • ChatPage keeps the readiness gate: operator handshake ΓåÆ tokenized chat HTTP probe ΓåÆ gateway chat.send ΓåÆ wait for the matching assistant final when a run ID is returned ΓåÆ navigate WebView.
  • BootstrapMessageInjector and its WebView composer injection scaffolding are gone; ChatWindow and the legacy onboarding chat overlay no longer dispatch post-navigation injection.
  • Added OnboardingChatBootstrapper tests for success, send failure, and completion timeout. Updated test counts: Shared 1465 total / 1443 passed / 22 skipped; Tray 909 total / 909 passed.
  • Validation passed: ./build.ps1; dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore; dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore -m:1.
  • Manual smoke: stopped prior tray process, built fresh exe, launched with OPENCLAW_VISUAL_TEST=1, confirmed startup stayed alive and log showed no new exceptions in the Option Z paths. Startup visual output dir: visual-test-output\option-z-startup.

Mike manual verification checklist

A. Tray auto-opens to Chat tab when wizard finishes.
B. Chat connects to gateway.
C. Hatching conversation appears as a real two-way thread: one user message and one claw response; composer is empty and send button is ready.
D. No second pairing notification.
E. wsl -d OpenClawGateway -- cat ~/.openclaw/devices/paired.json shows exactly one Windows-node entry.
F. WebView never shows a 404; it shows the waiting state until gateway/chat/bootstrap readiness completes, then loads chat.

Round 5 — H1 race fix per Hanselman review

  • Fix shape: subscribe-first plus buffer-replay in OnboardingChatBootstrapper; completion events are observed before chat.send, buffered by runId until the send response identifies the run, and the timeout is anchored at subscription time.
  • Regression: added BootstrapAsync_ConsumesGate_WhenCompletionArrivesSynchronouslyDuringSend, where the fake gateway raises the final assistant event synchronously inside SendChatMessageForRunAsync; it fails against the old send-first path and passes with the new observer.
  • Validation: ./build.ps1 passed; OpenClaw.Shared.Tests passed 1465 total (1443 passed, 0 failed, 22 skipped); OpenClaw.Tray.Tests passed 910 total (910 passed, 0 failed, 0 skipped). OPENCLAW_RUN_INTEGRATION was unset; OPENCLAW_REPO_ROOT was set to the worktree for final validation.
  • Reference: .squad/decisions/inbox/bostick-pr312-hanselman-review.md H1.

Mike Harsh and others added 6 commits May 11, 2026 07:52
…perly bootstrap hatching prompt

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…eService owns identity

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…bootstrap injector

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ector

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@shanselman

Copy link
Copy Markdown
Collaborator

Thanks for this — #312 is very close and CI is green. The core direction looks right: restoring the post-wizard chat handoff, preventing duplicate bootstrap sends, using the node-role token check, and making the connection-manager node-start predicate gateway-aware all look like useful fixes.

I did a deep pass against the current branch and I would like a small follow-up before merge, mostly around making the bootstrap one-shot gate harder to consume accidentally.

Main thing to tighten: sent should mean accepted/sent, not just clicked

In BootstrapMessageInjector.BuildInjectionScript, the script currently returns "sent" immediately after either:

form.requestSubmit();
return 'sent';

or:

button.click();
return 'sent';

Then InjectAsync treats that result as definitive and permanently flips the one-shot gate:

if (string.Equals(status, "sent", StringComparison.Ordinal))
{
    MarkInjected(settings);
    Logger.Info("[BootstrapMessageInjector] Bootstrap message injection sent");
    return true;
}

The risk is that requestSubmit() / button.click() can be ignored by the SPA or blocked by UI state: disabled send button, validation, async hydration not complete, focus/composition state, or a transient event handler failure. In that case the script still reports "sent", and we permanently set HasInjectedFirstRunBootstrap = true, so the hatching prompt will not retry.

Suggested fix options:

  1. Only return "sent" after observing an acceptance signal, such as composer cleared, transcript contains the bootstrap message, or a known post-submit UI state changed.
  2. If we cannot reliably confirm acceptance, return "rendered" after the blind submit/click and leave the gate open for a retry.
  3. If the blind click is intentionally considered sufficient, please add a comment explaining that tradeoff and why losing a retry is acceptable here.

The important invariant I want preserved is: the one-shot gate should only be consumed when we have high confidence the bootstrap message actually made it into chat.

Process-global s_inFlight may suppress the only successful attempt

The new s_inFlight guard is useful for preventing duplicate injection, but it is process-global and is acquired before the initial delay:

if (Interlocked.CompareExchange(ref s_inFlight, 1, 0) != 0)
{
    Logger.Info("[BootstrapMessageInjector] Bootstrap injection skipped because another injection is in flight");
    return false;
}

await Task.Delay(initialDelayMs).ConfigureAwait(true);

That means if ChatPage, ChatWindow, and/or OnboardingWindow all navigate around the same time, one caller wins and the others return false. If the winner fires too early and returns "not-rendered", "rendered", or "no-input", the later caller that might have succeeded has already been skipped until another navigation/retry event happens.

Suggested fixes:

  • Acquire the in-flight guard after the initial delay and after the post-delay gate re-check, so early/duplicate navigations are less likely to suppress a ready one.
  • Or queue one retry when the winning attempt returns a non-sent status.
  • Or leave the code as-is but add a comment documenting that this is an intentional tradeoff: preventing double-send is more important than maximizing a single navigation's success rate.

Small cleanup: dispose the visual-test DataReader

In ChatPage.CaptureVisualTestChatAsync, DataReader should be disposed:

var reader = new DataReader(stream);
await reader.LoadAsync((uint)stream.Size);

Suggested:

using var reader = new DataReader(stream);

This is visual-test-only, so not a merge blocker, but it is an easy cleanup.

What looks good

  • StartupSetupState.HasStoredNodeDeviceToken now checks the node-role token instead of the operator token. That matches the method name and closes a real setup-state bug.
  • GatewayConnectionManager accepting a gateway-aware node-start predicate is a clean extension point.
  • Keeping the gate open on "rendered" is the right direction.
  • The added tests around node-token setup state and manager-owned node suppression are valuable.

I think this can be mergeable after the bootstrap gate semantics are tightened or explicitly documented.

Mike Harsh and others added 6 commits May 11, 2026 13:33
…rving

Wait for the operator hello-ok boundary before allowing the ChatPage WebView to navigate, then probe the tokenized chat URL until the HTTP surface returns success. Keep the Chat tab open in a bounded waiting/retry state so post-wizard auto-launch still opens Chat without exposing a transient 404.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Strengthen bootstrap injection so a submit/click attempt returns sent only after the composer clears or the bootstrap text appears in a user-message-like transcript element. If acceptance is not proven within the bounded poll window, return unconfirmed and leave HasInjectedFirstRunBootstrap open for retry; transcript selectors are intentionally broad because no single stable live-DOM selector is guaranteed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rective

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep .squad state local so repo-hygiene can pass.
@shanselman

Copy link
Copy Markdown
Collaborator

I pushed a tiny maintainer follow-up commit to this branch: 3d099d7 chore: remove tracked squad artifacts.

No product code was changed in that commit. It only removes the tracked .squad files from Git so the repo-hygiene guard added in #314 can pass. .squad/ is now intentionally local-only state in this repo.

Thanks for the substantive onboarding/chat rework here — after CI reruns, I’ll re-check the native-chat/bootstrap changes separately from the hygiene cleanup.

@indierawk2k2

Copy link
Copy Markdown
Contributor Author

Bostick PR #312 Hanselman Dual-Model Review

  • PR: fix(onboarding): restore post-wizard handoff and chat connection; properly bootstrap hatching prompt #312 fix(onboarding): restore post-wizard handoff and chat connection; properly bootstrap hatching prompt
  • Branch/head: fix/bootstrap-injector-properly / e8b3c82
  • Base: origin/master
  • Method: Hanselman adversarial review with two independent reviewers over the same PR scope.
    • Reviewer 1: claude-opus-4.6 security/architecture lens.
    • Reviewer 2: gpt-5.3-codex correctness/edge-case lens.
  • Verdict: SHIP-WITH-FIXES
  • HIGH-consensus findings: 1

Scope reviewed

Reviewed PR #312 diff vs origin/master in C:\Users\mharsh\OneDrive - Microsoft\Desktop\OpenClawWindowsInstaller\openclaw-hatching-v2, including PR metadata and relevant changed files. Focus was ship-readiness only: correctness, security, races, data loss, broken UX contracts, and test coverage. No stylistic findings are included.

Both Models Agree -> HIGH consensus

ID Severity Category File:line Finding One-line fix Fix confidence
H1 HIGH correctness / race src/OpenClaw.Tray.WinUI/Services/OnboardingChatBootstrapper.cs:58-66, :93-123 BootstrapAsync sends chat.send and only then subscribes to AgentEventReceived / ChatEventReceived in WaitForRunCompletionAsync. If the gateway emits the final assistant/lifecycle event before the subscription is attached, completion is missed, the Chat page can wait up to 90 seconds, HasInjectedFirstRunBootstrap is not consumed, and a future attempt can duplicate the first-run hatching message. Register the completion listener before issuing SendChatMessageForRunAsync, or buffer run-completion events by runId in the gateway client so early finals cannot be lost. 90%

Only One Model Flagged -> LOW consensus

ID Flagging model Severity Category File:line Finding Disposition Fix confidence
L1 claude-opus-4.6 MEDIUM security src/OpenClaw.Tray.WinUI/Pages/ChatPage.xaml.cs:228-242, :315-319 The chat readiness probe GETs the same tokenized http://...?token=... URL that the WebView will navigate to when the gateway URL is ws://; repeated probes amplify exposure for non-local cleartext gateways. Low-consensus follow-up. This exposure is largely inherited from the tokenized WebView URL itself and is not the agreed ship blocker for this PR. Consider avoiding token-bearing probe requests over non-TLS or probing an unauthenticated health endpoint. 70%
L2 claude-opus-4.6 LOW edge-case src/OpenClaw.Tray.WinUI/Services/OnboardingChatBootstrapper.cs:11, :46-89 Static in-flight guard is process-wide. A second ChatPage opened during the first bootstrap wait is skipped, but the finally normally clears the guard. Informational; no ship-readiness action. 40%

Consensus synthesis

Both reviewers independently found the same completion-event race in the new gateway-side bootstrap path. I verified the code shape: SendChatMessageForRunAsync waits for the chat.send RPC response, then WaitForRunCompletionAsync subscribes to completion events afterward. The gateway client emits AgentEventReceived and ChatEventReceived directly from incoming event handling, with no visible replay buffer for runId, so an early final event can be lost. This directly affects the PR's primary contract: one-shot first-run bootstrap through the gateway without duplicate hatching prompts.

Cleanup checklist before merge

  • Fix H1 by subscribing/buffering before chat.send can complete with a runId.
  • Add a regression test where the fake gateway raises the final event synchronously during or immediately after SendChatMessageForRunAsync; expected result is success and HasInjectedFirstRunBootstrap == true.
  • Re-run required validation after the fix: ./build.ps1, shared tests, and tray tests.
  • Optional follow-up: reduce token exposure in repeated chat readiness probes for non-TLS gateway URLs.

Bostick review note

This is not a broad rejection of Option Z. The architecture of moving hatching bootstrap out of WebView DOM injection and into authenticated chat.send is directionally stronger. The remaining blocker is a narrow ordering race in the new completion proof path; once the listener/buffer ordering is fixed and covered by a regression test, I would expect the PR to move to SHIP.

Mike Harsh and others added 2 commits May 11, 2026 19:59
….send RPC

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@indierawk2k2

Copy link
Copy Markdown
Contributor Author

H1 is fixed in the new commits 82186f8 and 989b0b3.

I used the subscribe-first + buffer-replay shape: OnboardingChatBootstrapper attaches the completion observer before chat.send, buffers final events by runId until the send response arrives, and keeps the 90s timeout anchored at observer attachment. I also added the regression where the fake gateway raises the final assistant event synchronously inside SendChatMessageForRunAsync; the sanity check fails against the old send-first path and passes with the fix.

Final validation: ./build.ps1 passed; Shared.Tests 1465 total / 1443 passed / 0 failed / 22 skipped; Tray.Tests 910 total / 910 passed / 0 failed / 0 skipped. This addresses the H1 finding from .squad/decisions/inbox/bostick-pr312-hanselman-review.md.

Ref: #312 (comment)

@indierawk2k2

Copy link
Copy Markdown
Contributor Author

@steipete — PR ready for your review when you have a chance.

Summary of the journey: This PR went through 5 rounds of fixes for the post-wizard hatching prompt. The final state delivers all 6 verifications I tested manually against a clean WSL gateway install:

  • A — Tray auto-launches Chat tab when wizard finishes
  • B — Chat connects to gateway
  • C — Two-way hatching conversation visible, composer empty, send button ready
  • D — No double pairing notification
  • E — Exactly one Windows-node entry in ~/.openclaw/devices/paired.json
  • F — No 404 — brief spinner then chat default state

Architectural shift in the final round (Option Z): Bootstrap hatching message now goes through the authenticated gateway chat.send API instead of WebView2 DOM injection. BootstrapMessageInjector is deleted entirely. This was driven by the upcoming move to native chat — DOM injection isn't viable long-term, so we cut it now. Subscribe-first + buffer-replay observer pattern closes the H1 race that the dual-model (Hanselman) review surfaced.

Validation: ./build.ps1 passing, Shared.Tests 1443/1465 (22 skipped), Tray.Tests 910/910. CI repo-hygiene was red briefly due to leaked .squad/ files in a recent push — fixed in commit a81fd9d (git rm --cached, no history rewrite).

Other notes:

Happy to walk through any part of it. — Mike

Mike Harsh and others added 2 commits May 11, 2026 21:59
Master added AddCronJobAsync/UpdateCronJobAsync/RequestCronJobsAsync/

CronRunsUpdated to IOperatorGatewayClient. The bootstrap test fake

needs stub implementations to compile after merging master.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@shanselman
shanselman merged commit fd8ff7d into openclaw:master May 12, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants