Skip to content

fix(gui): restore OpenAI account setup with canonical recovery gates - #469

Merged
Ingwannu merged 7 commits into
lidge-jun:devfrom
Wibias:fix/issue-443-openai-recovery
Jul 26, 2026
Merged

Ingwannu merged 7 commits into
lidge-jun:devfrom
Wibias:fix/issue-443-openai-recovery

Conversation

@Wibias

@Wibias Wibias commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Restores OpenAI account setup when the built-in provider is missing or disabled (Accounts picker + Codex Auth enable path), including the Clash fake-IP DNS exception for the exact canonical OpenAI seed.
  • Closes the fix(gui): restore OpenAI account setup paths #445 review gap: disabled providers.openai rows must pass the same canonical adapter / baseUrl / authMode checks before UI recovery is offered, and disabled-only PATCH { disabled: false } cannot re-enable a malformed OpenAI row unchanged.
  • Based on a cherry-pick of fix(gui): restore OpenAI account setup paths #445 from @apple-ouyang (Ouyang Xingyuan) onto current dev, plus the maintainer-requested validation hardenings and regressions.

Thanks @apple-ouyang for the original recovery work in #445 — this PR cherry-picks that change and layers the canonical gates from review.

Fixes #443
Supersedes #445

Credits

Test plan

  • bun run typecheck
  • bun test tests/provider-payload.test.ts
  • bun test ./gui/tests/codex-auth-provider-enable.test.tsx
  • bun test tests/management-provider-validation.test.ts (includes noncanonical disabled re-enable rejection + canonical re-enable with missing mode fill)
  • bun run lint:gui
  • CI Cross-platform + React Doctor green
  • Maintainer security review (auth / provider-validation path)

Changelog note

Restore OpenAI account setup when the built-in provider is missing or disabled, with credits to @apple-ouyang (#445). Noncanonical disabled OpenAI rows no longer get a false recovery path.

Summary by CodeRabbit

  • New Features

    • Added recovery for missing or disabled built-in OpenAI access through the Accounts picker and Codex Auth page.
    • OpenAI can be re-enabled while preserving saved account mode and model settings.
    • Added clear progress, availability, and error messages across supported languages.
  • Bug Fixes

    • Improved validation and normalization for canonical OpenAI configurations, including compatible network resolution scenarios.
  • Documentation

    • Updated provider guides and configuration contracts in multiple languages with recovery behavior details.

@github-actions github-actions Bot added the bug Something isn't working label Jul 25, 2026
@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@Wibias, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 8 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: bca623ca-77f5-495b-86d6-a2ec57d03508

📥 Commits

Reviewing files that changed from the base of the PR and between 196a6a8 and 077a407.

📒 Files selected for processing (7)
  • gui/src/components/provider-catalog/ProviderCatalog.tsx
  • gui/src/i18n/zh.ts
  • gui/src/pages/Providers.tsx
  • gui/src/provider-payload.ts
  • src/server/management/provider-routes.ts
  • tests/management-provider-validation.test.ts
  • tests/provider-payload.test.ts
📝 Walkthrough

Walkthrough

This PR adds recovery for missing or disabled canonical OpenAI providers. The GUI can recreate or re-enable OpenAI, the server validates and normalizes canonical configurations, benchmark DNS handling supports Clash fake-IP addresses, and localized UI, tests, and documentation cover the flow.

Changes

OpenAI provider recovery

Layer / File(s) Summary
Provider state and enablement contracts
gui/src/provider-payload.ts, src/providers/openai-tiers.ts
Adds canonical OpenAI state detection, preset-based creation, disabled-provider re-enablement, stable typed errors, and an exported canonical base URL.
Canonical provider validation
src/lib/destination-policy.ts, src/server/management/provider-routes.ts
Allows benchmark DNS addresses only for canonical OpenAI and normalizes and validates disabled-only re-enablement.
GUI recovery and account flow
gui/src/pages/CodexAuth.tsx, gui/src/pages/Providers.tsx, gui/src/i18n/*
Adds OpenAI recovery actions, busy and error states, account-picker inclusion, configuration refreshes, and translated messages.
Recovery behavior tests
gui/tests/*, tests/provider-payload.test.ts
Covers banner states, provider creation, re-enablement, request sequences, and preset-load failures.
Validation coverage and documented contract
tests/destination-policy-resolved.test.ts, tests/management-provider-validation.test.ts, structure/08_openai-provider-tiers.md, docs-site/src/content/docs/*/guides/providers.md
Tests canonical destination exceptions and PATCH normalization, and documents the gated recovery behavior.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant CodexAuth
  participant ensureOpenAiProvider
  participant ProviderRoutes
  participant DestinationPolicy
  User->>CodexAuth: select Enable OpenAI
  CodexAuth->>ensureOpenAiProvider: recover absent or disabled provider
  ensureOpenAiProvider->>ProviderRoutes: create or re-enable openai
  ProviderRoutes->>DestinationPolicy: validate canonical destination
  DestinationPolicy-->>ProviderRoutes: accept or reject destination
  ProviderRoutes-->>CodexAuth: refreshed provider state
  CodexAuth-->>User: show Pool mode or translated error
Loading

Possibly related PRs

  • lidge-jun/opencodex#445: Implements the same OpenAI account recovery and provider enablement flow across the GUI and backend.

Suggested reviewers: lidge-jun, ingwannu

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.76% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: restoring OpenAI account setup with canonical recovery gates in the GUI.
Linked Issues check ✅ Passed The PR matches #443 by restoring OpenAI visibility, recovery, canonical re-enable behavior, Codex Auth recovery, and fake-IP validation.
Out of Scope Changes check ✅ Passed The changes stay focused on OpenAI recovery, validation, UI, docs, and tests, with no unrelated feature work evident.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@lidge-jun

Copy link
Copy Markdown
Owner

🔒 Under maintainer review — detailed feedback incoming

@lidge-jun (maintainer) has this PR in an active review pass. Please do not merge, rebase, or
close it
until the detailed review lands; a full comment with specific file:line findings,
failure modes, and suggested fixes is being prepared.

This is a claim marker so two maintainers do not review or land the same PR at once. If you are a
maintainer and need this PR urgently, please reply here first instead of merging.

Baseline for this pass: dev is at bef0dfbe (yesterday's integration batch ebc62d1f plus
#458, #456, #391, #392, #463, #465). If this PR touches the same files, the upcoming review will
name the exact overlapping paths so you only have to rebase once.

No action needed from you until then. Thanks for the contribution and for your patience.

Review tracker: devlog/_plan/260726_pr_review_pass · marker posted by the maintainer review pass

@lidge-jun

Copy link
Copy Markdown
Owner

Thank you for hardening the OpenAI account setup recovery. I reviewed this against #445, which addresses the same problem, and this is the version we should take.

Duplicate resolution: take #469, close #445. The patch IDs of the original change match, so #445's authorship is preserved here, and this PR adds the base-URL gate, server-side re-enable validation, and regressions for malformed disabled rows that #445 is missing. #445 in its current form allows a disabled row with a hostile baseUrl to be re-enabled and then receive forwarded account credentials; I've written that up on that PR separately.

Verdict: BLOCKED_SECURITY — policy gate, not a defect. gui/src/provider-payload.ts:121 adds automatic creation and re-enablement of the built-in OpenAI provider, and src/server/management/provider-routes.ts:238 changes the server-side authentication recovery gate. Per MAINTAINERS.md that requires an explicit, recorded maintainer security review before merge. I want to be clear that this is a process requirement rather than a finding against your code: I could not find a remaining credential-destination bypass in this implementation.

What I verified holds: enabled and disabled rows both require adapter=openai-responses, authMode=forward, and the normalized canonical ChatGPT Codex URL; the server independently rejects noncanonical disabled rows before re-enabling them; and the canonical POST is still covered by the stricter provider-seed validator.

Overlap with dev: the branch is two commits behind the bef0dfbe baseline (c03ed0a3, bef0dfbe). A synthetic merge shows no textual conflict, but please rebase before merge so the security review is recorded against the exact final head. GitHub currently reports CLEAN with the full matrix, npm-global, React Doctor, target, and label checks passing.

Test gap: the new GUI test only asserts banner markup. It never clicks the recovery action, so the absent-provider POST, disabled-provider PATCH, error display, and post-success refresh are unproven end to end. One interaction-level regression through the rendered page would close the most important journey here. Backend and payload tests already cover the main canonical/noncanonical branches.


Reviewed as part of a maintainer review pass against dev at bef0dfbe, snapshot 2026-07-26T08:35+09:00, at head 3a71a37a.

apple-ouyang and others added 4 commits July 26, 2026 01:47
原因:
- OpenAI account credentials can outlive the built-in provider, leaving no GUI path to restore routing.
- Disabled legacy or API-key OpenAI rows must not be treated as canonical Codex account providers.
- Clash fake-IP DNS can reject the exact canonical OpenAI preset after canonical seed validation.

改动:
- Keep OpenAI visible in the unified account picker and add in-place recovery on Codex Auth.
- Create missing canonical providers, re-enable only canonical disabled providers, and reject noncanonical rows without overwriting them.
- Preserve destination checks for custom providers and cover provider-state, banner-state, test isolation, and fake-IP boundaries.
Cherry-picked #445 from @apple-ouyang (Ouyang Xingyuan) onto current
dev, then require the same adapter/baseUrl/authMode checks for disabled
rows in Codex Auth and reject noncanonical openai re-enable via PATCH.

Co-authored-by: Ouyang Xingyuan <galaxyroaming@qq.com>
Keep destination checks for canonical openai and only suppress
benchmark/fake-IP answers; normalize baseUrl when re-enabling; localize
enable failures; sync structure and provider docs with gated recovery.

Co-authored-by: Ouyang Xingyuan <galaxyroaming@qq.com>
Prove absent→POST, disabled→PATCH, localized error display, and
post-success Pool refresh through the rendered page, not banner markup alone.
@Wibias
Wibias force-pushed the fix/issue-443-openai-recovery branch from 5beb38a to b77c43e Compare July 25, 2026 23:50
@Wibias

Wibias commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the review follow-ups on head b77c43ee:

  • Rebased onto current dev (8756daa5, includes the bef0dfbe baseline) so security review can be recorded against the final head.
  • Interaction test added in gui/tests/codex-auth-recovery-interaction.test.tsx: clicks Enable OpenAI for absent→POST, disabled→PATCH, localized preset-load error alert, and post-success Pool refresh.
  • fix(gui): restore OpenAI account setup paths #445 is already closed as superseded; authorship remains via the cherry-pick + Co-authored-by credits.

Still waiting on the explicit maintainer security review (BLOCKED_SECURITY process gate) before merge — understood this is policy, not a defect finding against the implementation.

Wibias added 2 commits July 26, 2026 01:57
Allow only Clash 198.18/19 answers inside destination resolution for
the canonical seed, normalize baseUrl on re-enable with save-and-reload
coverage, and always localize enable failures through t(...).
PATCH {disabled:false} for openai now resolves destinations with the
canonical base URL and allowBenchmarkAddresses, ignoring any persisted
allowPrivateNetwork so Clash fake-IP alone can pass.
@Wibias

Wibias commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

Ready for merge. @lidge-jun

coderabbitai[bot]

This comment was marked as resolved.

Guard Accounts enable against double-clicks, localize the missing-seed
path, unify zh provider wording, and drop stale allowPrivateNetwork after
a successful disabled-row re-enable.

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Reviewed the exact head 077a407a45c6399dcd922dfadde3e79f82a4ceeb against the current dev merge result.

The recovery flow is correctly gated to the canonical OpenAI adapter, forward auth mode, and normalized Codex base URL. Missing providers are created only from the canonical server preset; disabled providers are re-enabled through the restricted PATCH path; noncanonical rows remain blocked. The destination-resolution exception is limited to Clash benchmark answers and still rejects loopback, RFC1918, metadata, and mixed dangerous results. The successful recovery path also removes stale allowPrivateNetwork.

Verified locally:

  • 71 focused provider-management, destination-policy, and payload tests
  • 6 GUI recovery component/interaction tests, including actual POST/PATCH clicks, error display, and post-success refresh
  • bun run typecheck
  • bun run lint:gui

All current GitHub checks are green. The CodeRabbit duplicate-declaration report is not present on this head (each test has one declaration), and the remaining canonicalization-helper suggestion is non-blocking maintainability follow-up.

@Ingwannu
Ingwannu merged commit b59fc00 into lidge-jun:dev Jul 26, 2026
9 checks passed
@Wibias
Wibias deleted the fix/issue-443-openai-recovery branch July 27, 2026 03:53
agentHits pushed a commit to agentHits/opencodex that referenced this pull request Sep 17, 2026
…overy

fix(gui): restore OpenAI account setup with canonical recovery gates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants