fix(setup): harden completion + reconnect, refresh header - #146
Conversation
- credentials: probe new URL with progressive backoff (~52s) instead of one-shot 5s, show soft "reconnecting" toast rather than red error when the hotspot reconfig / hostname change drops the tab. Backoff timer is cancellable on abort to avoid setState-after-unmount. - completion: retry POST /setup-api/setup/complete up to 3x with backoff to absorb gateway-restart timing from the AI step. Add 15s per-attempt timeout. Friendly i18n error with a "retry" button instead of raw "Failed to fetch" (10 locales). - header: swap icon to the openclawhardware.dev brand crab; restyle version badge inline next to title with muted slash separator. - internal: hoist t into a ref so locale switches during an in-flight completion don't trigger a duplicate POST.
|
Warning Review limit reached
More reviews will be available in 45 minutes and 38 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds backoff retry probing for mDNS after credential saves, implements a retrying, timeout-bound POST for setup completion, maps transient failures to localized unreachable messages, compacts the header layout, adds a step-5 retry button, updates translations across locales, and tolerantly dismisses a modal in an E2E test. ChangesSetup Wizard Network Resilience
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/CredentialsStep.tsx`:
- Around line 203-206: Replace the hardcoded English reconnect messages used in
the setStatus calls with calls to the existing i18n function t(...), e.g.,
change the strings that reference newSetupUrl.toString() to use t with an
interpolation placeholder; update both places where setStatus is called with
messages containing the URL (the calls that build a "Settings saved.
Reconnecting to …" style message) and add corresponding translation keys to your
locale files so non-English locales render correctly while still passing
newSetupUrl as a variable to t.
- Around line 219-223: The MDNS probe uses AbortSignal.timeout so it can't be
cancelled by the existing save/unmount AbortController; change the probe to use
the active controller (controller.signal) and implement a timeout that aborts
that controller (e.g., setTimeout(() => controller.abort(),
MDNS_PROBE_TIMEOUT_MS)) before calling fetch, pass controller.signal to fetch,
clear the timeout after the request, and after awaiting the fetch check
controller.signal.aborted and skip the window.location.replace(...) if aborted;
reference the existing controller variable, MDNS_PROBE_TIMEOUT_MS, the fetch
call doing method "HEAD", and the subsequent window.location.replace call to
locate where to apply these changes.
In `@src/components/SetupWizard.tsx`:
- Around line 394-414: postCompleteWithRetry currently returns the first fetch
Response even when it's an HTTP error (res.ok === false); change
postCompleteWithRetry to inspect the fetched Response from
fetch("/setup-api/setup/complete") and only return it when res.ok is true,
otherwise treat it like an error (store it or throw) so the retry loop
(backoffs) will continue; on final failure after all backoffs, throw a
meaningful Error that includes the last response status/text or the caught error
to aid debugging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 18d9c546-9517-4784-9a9e-39e940a86b8e
⛔ Files ignored due to path filters (1)
public/clawbox-icon.pngis excluded by!**/*.png
📒 Files selected for processing (3)
src/components/CredentialsStep.tsxsrc/components/SetupWizard.tsxsrc/lib/translations.ts
- credentials: i18n the reconnect "saved/reconnecting" and "still
unreachable" status messages (added credentials.reconnecting +
credentials.reconnectFailed across 10 locales with {url} placeholder)
- credentials: probe fetch now uses AbortSignal.any([controller.signal,
AbortSignal.timeout(...)]) so it cancels on save/unmount abort, not
just on per-attempt timeout. Added post-fetch + post-catch
controller.signal.aborted checks to skip the redirect after a late
abort.
- completion: postCompleteWithRetry now treats non-OK HTTP responses as
retryable (a transient 5xx during gateway-restart settle no longer
short-circuits the backoff). On final failure includes the last
response status/body in the thrown Error for debug.
CI e2e-install/80-chat.spec.ts started failing on the Send-button click because the upstream ClawAI proxy now reports a paid tier on fresh dashboard loads, which pops the TierUpgradeCelebration dialog (z-[100001]) on top of the chat panel and intercepts pointer events. Defensive dismiss: between fill() and the Send click, look for the modal's "Let's go" button with a 3s timeout. If no modal appears, the catch absorbs the timeout silently. This is purely a test robustness fix — the celebration is correct product behaviour for a Free→Paid transition.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Summary
Post-v3.0.5 hardening for the setup wizard. Three concerns bundled because they all surface around the late-wizard steps (3/5) and share user-impact (the customer sees an unrecoverable red error in cases where setup actually succeeded or would have succeeded with a brief retry).
1. Credentials step (step 3) — reconnect probe
After the hotspot/hostname change, the browser's connection to the device drops while avahi re-broadcasts and the WiFi AP cycles. The previous one-shot 5 s HEAD probe was too aggressive — users saw a red "Could not reach" error even when the device came back ~8 s later.
2. Setup completion (step 5) — retry + friendly error
POST /setup-api/setup/completecan fail transiently when the gateway restart from the AI step is still settling, or the browser's connection is briefly degraded after the step 3 hotspot reconfig. Until now this produced a dead-end "Failed to fetch" banner with no recovery path.AbortSignal.timeoutso a hung request doesn't block the full backoff windowwizard.completionUnreachable) added for all 10 locales — replaces raw browser error textthoisted into a ref so a mid-completion locale switch can't re-fire the effect and double-POST3. Header refresh
Files
postCompleteWithRetry, friendly error mapping, retry button, version badge restyle, `t` refwizard.completionUnreachablefor en/bg/de/es/fr/it/ja/nl/sv/zhTest plan
bun run build)Summary by CodeRabbit
Bug Fixes
Improvements
Tests