Skip to content

fix: don't show onboarding when server is unreachable#8312

Open
jeffa-block wants to merge 2 commits intoblock:mainfrom
jeffa-block:jeffa/fix-onboarding-guard-fail-open
Open

fix: don't show onboarding when server is unreachable#8312
jeffa-block wants to merge 2 commits intoblock:mainfrom
jeffa-block:jeffa/fix-onboarding-guard-fail-open

Conversation

@jeffa-block
Copy link
Copy Markdown
Contributor

When readConfig fails (goosed down), OnboardingGuard falls through to setHasProvider(false), showing the Welcome screen to existing users. This is confusing — they think they need to reconfigure.

Fix: Persist a localStorage flag (goose_has_provider) on successful provider detection. On config read failure, check the flag before falling through to onboarding. Existing users see the error screen instead of Welcome.

Fixes #8311 (PR 1 of 2)

Changes

  • OnboardingGuard.tsx: persist flag on success; check it on failure before showing onboarding (+9/-2)
  • OnboardingGuard.test.tsx: 6 tests covering success path, fail-open for existing users, first-time user fallback, and edge cases (+149)

Testing

  1. Configure a provider normally → localStorage flag is set
  2. Kill goosed → navigate to / → see error screen, NOT Welcome
  3. First-time user (no flag) → still sees Welcome as expected
  4. pnpm vitest run src/components/onboarding/OnboardingGuard.test.tsx → 6/6 pass

When goosed is down, OnboardingGuard's config read fails and
falls through to setHasProvider(false), showing the Welcome
screen to existing users. This is confusing — they think they
need to reconfigure.

Fix: persist a localStorage flag on successful provider detection.
On config read failure, check the flag before falling through to
onboarding. Existing users see the error screen instead of Welcome.
Tests the three key paths:
- Provider detected → localStorage flag set, children rendered
- Config read fails + flag exists → existing user sees children (not onboarding)
- Config read fails + no flag → new user sees Welcome screen

Includes a localStorage shim for the jsdom environment (getItem/setItem
are undefined in this repo's test setup).
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ff63960396

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +43 to +45
if (configured) {
localStorage.setItem('goose_has_provider', 'true');
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clear cached provider flag when config is empty

goose_has_provider is only ever written to 'true' and never removed when GOOSE_PROVIDER becomes empty, so the cache can go stale. A user can reset provider config (see ResetProviderSection removing GOOSE_PROVIDER) and still retain this flag; if read() later fails (for example, goosed is unreachable), the catch path treats them as configured and skips onboarding even though no provider is configured. Clear or remove the localStorage flag when configured is false to keep fallback behavior aligned with current config.

Useful? React with 👍 / 👎.

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.

fix: improve disconnect recovery UX — don't show onboarding to existing users

1 participant