Fix onboarding's credential validation and mid-session provider connect - #405
Merged
Merged
Conversation
TheGreatAxios
force-pushed
the
cl-5687-5688-5689-5690-onboarding-fixes
branch
from
August 8, 2026 19:31
1d9360e to
dd527d5
Compare
Trying a model in the picker recorded it in recentModels but also carried defaultProvider into the same settings write, so browsing a model became an implicit "set as default" — every model tried in a session silently became the new default for future launches.
An empty API key on a key-required preset was written as keyless: true, and resolveProvider skips the missing-key check entirely once keyless is set, so a config that should have failed validation loaded as configured and only broke on the first real send. The submit funnel now rejects an empty key on any preset that isn't genuinely keyless-capable (the custom/manual endpoint), instead of downgrading it. The "save anyway" escape hatch had the same failure shape one step further along: a credential that failed its connection test still got persisted, indistinguishable from a verified one. It's now marked verified: false, and the running session surfaces a one-time startup notice pointing back to setup instead of letting the first send fail with a raw adapter error. Both bugs existed twice over: first-run onboarding and the mid-session "connect a new provider" flow each reimplemented the same submit logic, so a fix to one alone would have left the other silently broken. The submit logic moves into its own module and both callers now share it.
An empty API key on a key-required preset was written as keyless: true, and resolveProvider skips the missing-key check entirely once keyless is set, so a config that should have failed validation loaded as configured and only broke on the first real send. The submit funnel now rejects an empty key on any preset that isn't genuinely keyless-capable (the custom/manual endpoint), instead of downgrading it. The "save anyway" escape hatch had the same failure shape one step further along: a credential that failed its connection test still got persisted, indistinguishable from a verified one. It's now marked verified: false, and the running session surfaces a plain-language startup notice instead of letting the first send fail with a raw adapter error. Both bugs existed twice over: first-run onboarding and the mid-session "connect a new provider" flow each reimplemented the same submit logic, so a fix to one alone would have left the other silently broken. The submit logic moves into its own module and both callers now share it. verified defaults to trusted (absent, not false) so this doesn't retroactively flag every existing user's already-working setup; only a path that persists a credential without testing it sets it false. The OAuth submit path is exempt from the same marking on purpose — a completed OAuth login is a stronger signal than a pasted key (the provider's own server just issued it), though it doesn't confirm the resulting token carries API scope, which is tracked as a separate follow-up rather than faked here.
TheGreatAxios
force-pushed
the
cl-5687-5688-5689-5690-onboarding-fixes
branch
from
August 8, 2026 21:20
dd527d5 to
76c24c1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two of the four bugs in the onboarding cluster, landed as separate commits:
src/tui/runner.ts): trying a model to browse it was silently rewritingdefaultProviderfor future launches, not just updating the recents list.src/tui/onboarding.ts, newsrc/tui/provider-setup-submit.ts,src/tui-opentui/provider-connect.ts): an empty API key on a key-required preset was downgraded tokeyless: true, andresolveProviderskips the missing-key check once that's set — so a broken config loaded as "configured" and only failed on the first real send. The "save anyway" bypass had the same shape: an unvalidated credential got persisted indistinguishably from a verified one. The submit logic (empty-key rejection, connection test,verified: falsemarking) now lives in one module both first-run onboarding and the mid-session "connect a new provider" flow share, instead of three separate copies of the same logic.Not included: CL-5687 ("connect a new provider" mid-session)
This turned out to already have a fix on
main(connectProviderInline/provider-connect.ts, landed independently). Live-testing that fix in a real terminal (not just the headless test harness) found it still crashes with "stdin is already used by another CliRenderer" — a secondCliRenderercan't share the terminal with the session's live one.I built a fix (reuse the host's own renderer, paused for the duration) that resolved the crash, but further live testing surfaced a worse regression: after the sub-flow completed, the host's prompt began double-inserting every keystroke ("x" became "xx"). I could not root-cause this confidently within scope, and shipping a fix that corrupts the live session's own input afterward is worse than the known crash. I'm not including that commit here. Filed for follow-up separately from this PR rather than shipping something I verified was broken.
Verification
Both included flows were driven in a real terminal (tmux + a fresh
$HOME/settings fixture), not just the test suite:verified: false, and the session shows a plain-language startup notice instead of a raw adapter error.defaultProvideris left untouched.Test plan
bun run typecheckbun run buildbun run test(full suite, 4177 pass)