fix(ui): recover first-run finish errors without looping - #11906
Conversation
…a finish error
The in-chat first-run wizard could loop forever with no escape. On any
finish/provision error, `seedError` re-appended the runtime CHOICE
(`${message}\n\n${RUNTIME_CHOICE}`), so a persistent failure — e.g. the
"Not found" 404 from `POST /api/first-run` — re-offered the same runtime
question indefinitely with no distinct error surface and no way out. The
"Other / configure in Settings" provider pick made it worse: it ran a local
finish that hit the same 404 and re-looped instead of ever opening Settings.
This is the UX/navigation fix (the underlying 404 is a separate backend/env
issue, out of scope):
- Finish errors now seed a DISTINCT, non-looping recovery turn
(`first-run:error:*`) with a human message and a dedicated
`[CHOICE:first-run id=error]`: Try again (`error:retry`, re-runs the last
runtime's finish), Choose a different way to run (`error:restart`, re-offers a
fresh unlocked runtime CHOICE), and Configure in Settings (`error:settings`).
- "Other / configure in Settings" (`provider:other`) now opens the Settings tab
(`setTab("settings")`) and exits first-run (`completeFirstRun("settings")`)
via a shared `exitToSettings` helper, latched by `completedRef` so a
double-tap can't flip the gate twice.
- Raw terse errors ("Not found", "Failed to fetch", …) are wrapped in a clear,
human sentence via `finishErrorMessage`.
Local-success, cloud, and needs-cloud-login paths are unchanged.
Tests: updated the affected conductor tests and added coverage for the
persistent-404 no-loop + retry + Settings escape, the cloud `error:retry`
re-run, and `provider:other` -> Settings exit. Also hardened `beforeEach` to
restore default mock implementations (clearAllMocks keeps implementations, so a
leaked mockRejectedValue would poison later tests). Full src/first-run suite:
191 passed.
Closes #11882
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Supersedes #11884 with the same first-run recovery fix rebased on current
develop, preserving the already-merged cloud-login resume behavior from #11899.Summary
provider:otherexits first-run into Settings instead of running a local finish flow that can fail and trap the user.developwhile adding the fix(ui): make first-run onboarding recoverable instead of looping on a finish error #11884 recovery behavior.Validation
Run from
/home/shaw/eliza-worktrees/pr-11884-first-run:bun run --cwd packages/core prebuild— passbun run --cwd packages/ui test src/first-run/— pass, 16 files / 199 testsbun run --cwd packages/ui typecheck— passbunx @biomejs/biome check packages/ui/src/first-run/IN_CHAT_ONBOARDING_DESIGN.md packages/ui/src/first-run/use-first-run-conductor.test.ts packages/ui/src/first-run/use-first-run-conductor.ts— passgit diff --check origin/develop...HEAD— passI could not push the rebase back to
roninjin10/eliza:fix/first-run-loop-configure-settings; GitHub rejected the fork push withAuthentication required: You must have push access to verify lockseven though the original PR has maintainer edits enabled.