Simplify provider setup with unified OAuth and API panels#247
Simplify provider setup with unified OAuth and API panels#247HaydernCenterpoint wants to merge 1 commit into
Conversation
|
Hey @HaydernCenterpoint thanks for the PR. I do not think this PR achieves its stated goal of consolidating provider setup into one approachable flow. It adds two summary panels above the existing provider list while retaining the Workspace/Classic split, the existing Add Provider modal and a separate OAuth account dialog. This increases the number of provider representations and entry points rather than reducing them. The primary information architecture is also backwards. Users should select a provider first and then choose from the authentication methods that provider supports. Requiring users to decide between “OAuth” and “API Providers” upfront contradicts the stated goal of making setup usable without understanding authentication modes. These categories are not mutually exclusive either, since some providers support both OAuth and API-key configuration. Configured providers must remain availableHiding already configured providers is a blocker. A configured provider is not necessarily complete. Users may need:
The current implementation also compares configured provider names with preset IDs. This means a canonical configuration name may hide the preset while a custom-named configuration for the same provider does not. The resulting behaviour depends on naming rather than actual provider capabilities. Configured providers should remain visible and show their current state. Selecting one should offer actions such as:
If multiple configurations are technically unsupported for a particular provider, the UI should explain that limitation rather than silently removing the provider. Free providers are missingThe redesigned flow removes the existing Free provider category without providing an equivalent replacement. When the API-provider picker is opened in API-key-only mode, the Accounts, Free and Paid tabs are hidden. The catalogue is then restricted to providers that support API-key setup. Free providers using forwarding, local, keyless or other non-API-key setup methods are therefore excluded. The OAuth picker only covers account-login providers, so it does not restore those missing options. This is a functional regression, not merely an information-architecture preference. Free providers are particularly important for onboarding users who want to try OpenCodex without first obtaining a paid API key or connecting a subscription account. There are only two possible outcomes:
OAuth and API key are not an exhaustive taxonomy of supported provider setup methods. The replacement flow must support free, forwarding, local, keyless and custom configurations before it can replace the existing Add Provider flow. The panels duplicate existing managementThe two summary panels mostly repeat information already shown by the complete provider cards below them. The panels show configured providers and simple status information, while the actual management actions remain in the original cards. The page therefore contains:
This is more interface, not less interface. The panels also permanently reserve the most prominent part of the page for low-frequency setup actions. An API-key-only user receives an empty OAuth panel. An OAuth-only user receives an empty API-provider panel. Those empty sections imply incomplete setup even when the user has deliberately configured everything they need. Sync and restart need an explicit state flowReplacing the header-level Add Provider action with Restart Codex can make sense because provider additions are now initiated through the panel controls. However, the automatic synchronisation behaviour and restart messaging are not well designed. The current implementation automatically runs model synchronisation after adding a provider or completing a new OAuth connection. That combines three separate operations:
These operations can succeed or fail independently and should not be represented as one vague success message. I would use the following flow:
The restart prompt should appear after synchronisation succeeds, not before it begins. If synchronisation fails, the UI should show a retry action rather than recommending a restart. If synchronisation produces no changes, there is no reason to interrupt the user with a restart prompt. The header-level Restart Codex action can remain available as a manual action, but it should reflect state. It should become visually prominent when a restart is required rather than relying on a temporary notification that users may miss. The implementation adds special-case complexityThe For providers whose primary authentication method is not an API key, the implementation clones the preset and replaces its authentication value with A cleaner structure would be:
The PR description understates the scopeThe checklist describes this as a localised UI-only flow, but the PR changes application restart behaviour, the management API, provider derivation, authentication capabilities and model synchronisation. That is not merely a visual rearrangement. These behaviours require clear documentation and tests covering the actual user workflows, not only rendering snapshots and type checks. Recommended directionI would retain the useful lower-level work where appropriate, but revise the flow around the following principles:
@lidge-jun In my opinion the two permanent panels and the restricted provider picker should not be merged in their current form. |
|
@Wibias Thanks for the preview, i will working on it. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
opencodex/gui/src/pages/Providers.tsx
Line 500 in b498b94
Keep missing-key providers setup-able
With this filter, a key-auth provider that exists but does not yet have a key (for example, an API provider added while leaving the key blank, which the modal permits, or an existing openai-apikey missing its key) is omitted from the API Providers panel. The Add API Provider modal is also passed the existing names as exclusions, so the same provider cannot be selected again to supply the key, and the classic view has no add-key controls for rows without a key pool; users are stuck unless they switch to workspace or edit JSON. Include key-auth providers that need setup here and show the key-add action.
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <div className="row"> | ||
| <button className="btn btn-ghost btn-sm" onClick={toggleWorkspace}>{t("pws.classicToggle")}</button> | ||
| <button className="btn btn-primary" onClick={() => setAdding(true)}><IconPlus />{t("prov.add")}</button> | ||
| {restartCodexButton} |
There was a problem hiding this comment.
The workspace header now renders only the restart button where the Add Provider action used to be. In the non-empty workspace, ProviderWorkspaceShell does not render any add control (its onAddProvider entry points are only in the empty state), so users who already have at least one provider cannot add another provider from workspace view without switching views or editing JSON. Keep the add-provider action and add restart as a separate control.
Useful? React with 👍 / 👎.
| ) : ( | ||
| <> | ||
| <button className="btn btn-primary" onClick={() => setAdding(true)}><IconPlus />{t("prov.add")}</button> | ||
| {restartCodexButton} |
There was a problem hiding this comment.
In classic view this replaces the generic Add Provider button with only Restart Codex and Edit JSON. The new summary tiles open account-only or API-key-only pickers, so non-account/non-key presets such as local Ollama/LM Studio or keyless free providers no longer have a UI path from classic view once the config is non-empty. Keep the full catalog Add Provider action available alongside restart.
Useful? React with 👍 / 👎.
| ) : providerBusy ? ( | ||
| <button className="btn btn-ghost btn-sm" onClick={() => void cancelLoginOAuth(row.id)}>{t("common.cancel")}</button> | ||
| ) : providerNeedsReauth ? ( | ||
| <button className="btn btn-primary btn-sm" onClick={() => requestLoginOAuth(row.id, true, activeAccount?.id)}>{t("prov.reauthenticate")}</button> |
There was a problem hiding this comment.
When a connected OAuth provider needs reauth and the user starts it from this summary row, loginInfo and the manual redirect-code controls are not rendered here; those controls now exist only under the account picker’s visibleAccountRows, while connected/reauth rows are filtered into connectedAccountRows. In browser-open or localhost-callback failure scenarios, users cannot copy the auth URL, cancel from the row, or paste the redirect code for the reauth flow. Render the same login hint/fallback for connected rows while they are busy.
Useful? React with 👍 / 👎.
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
The provider setup direction is valuable, and I found no malware or credential-exfiltration behavior. However this large branch conflicts with current dev in Providers.tsx and management-api.ts, has no full test/build matrix, and adds a security-sensitive local process-control endpoint for restarting Codex. Please rebase, resolve the current provider/auth changes, and run the full CI matrix before this can merge. Keep the restart endpoint bounded to fixed platform commands and the existing origin-checked management surface.
|
@Ingwannu Please have a look at my comment. There are many other problems with that pr. |
|
@Wibias Thanks for flagging this. I checked the current The current implementation still has concrete functional and information-architecture regressions:
These are blocking product regressions, not styling preferences. The PR should remain |
Summary
+pickers provide search and hide providers that are already set up.The goal is to converge on one approachable provider UI. Many OpenCodex users will not be familiar with provider configuration, so a searchable, guided setup flow is easier to understand than multiple competing views.
This supersedes #226 and addresses its review feedback: the branch now targets
dev, Russian locale keys are included, restart waits for real platform completion and propagates failures, and the UI coverage renders React output instead of checking source strings.Verification
bun test --isolate ./tests/codex-app-restart.test.ts ./tests/provider-auth-ui-render.test.ts ./tests/provider-workspace-data.test.ts ./tests/provider-registry-parity.test.ts— 80 pass, 2 snapshotsbun run typecheckcd gui && bun run lintcd gui && bun run buildbun run privacy:scannode gui/scripts/sync-locale-keys.mjs— de/ko/zh/ru up to dateprintfthroughcmd.exe(no Cursor files are changed here).Checklist