fix(web): resetting managed Codex keeps managed setup#28895

open5

main

fix/issue-28885

View on GitHubTwo-way sync enabledPR last imported 2d ago

Problem

In Settings → Providers, "Reset to default" on the default Codex instance deleted the whole providerInstances.codex entry, including setupMode: "managed". The card then fell back to existing-CLI fields with no managed/existing switch, so Codex silently ran whatever codex was on PATH (or nothing) and Settings offered no way back. The button also showed straight after a managed onboarding, because any explicit default instance counted as changed.

Reproduced on main 4d7cb9bc2e in the web dev server: seed settings.json with the managed Codex entry onboarding writes, open Settings → Providers → Codex, click "Reset Codex provider settings to default". providerInstances.codex disappears and the card switches to Binary path / CODEX_HOME fields.

Change

defaultInstanceResetBaseline in ProviderSettingsPanel.logic.ts gives the state a default instance resets to. For Codex in managed setup it is the stored onboarding shape { driver: "codex", enabled: true, config: { setupMode: "managed" } } (the server folds config.enabled into enabled, so this is what onboarding leaves on disk). Every other default instance, including existing-mode Codex, has no baseline and still resets by removal.

The panel uses that one helper twice: reset upserts the baseline when there is one (otherwise removes as before), and an instance equal to its baseline is not dirty, so the reset button stays hidden for an untouched managed install.

Scope and approval

Fixes pingdotgg/t3code#28885

Verification

Focused tests, in ProviderSettingsPanel.environment.browser.test.tsx:

resets an edited managed Codex instance to managed setup instead of removing it: fails on main (operation: "remove"), passes here.
does not offer reset for a managed Codex instance still at its onboarding config: fails on main (button rendered), passes here.
resets an existing-CLI Codex instance by removing it: guard, passes on both.

vp run check:changed on 909ad8b167: format, lint, knip, typecheck, tests and React compiler pass.

Runtime transcript, real web dev server with a fresh database per side (T3CODE_DEV_SEED=0), each fixture written to the server's settings.json, reset clicked in the browser, stored entry read back from settings.json:

1	## main 4d7cb9bc2e — M1 (edited managed, enabled)
2	seeded: {"driver": "codex", "enabled": true, "displayName": "Personal", "config": {"setupMode": "managed", "launchArgs": "--verbose"}}
3	UI before click: resetButton=true managedSetup=true existingRuntimeFields=false (title "Personal", "Not authenticated · Sign in with ChatGPT")
4	UI after click: resetButton=false managedSetup=false existingRuntimeFields=true (title "Codex", "Authenticated · OpenAI API Key" from the codex CLI on PATH)
5	stored after: {"codex": null, "claudeAgent": null}
6
7	## main 4d7cb9bc2e — M0 (raw onboarding write, config.enabled form)
8	seeded: {"driver": "codex", "config": {"enabled": true, "setupMode": "managed"}}
9	UI: resetButton=true managedSetup=true

1	## head 7534b06e14 — M1 (edited managed, enabled)
2	seeded: {"driver": "codex", "enabled": true, "displayName": "Personal", "config": {"setupMode": "managed", "launchArgs": "--verbose"}}
3	UI before click: resetButton=true managedSetup=true existingRuntimeFields=false (card title "Personal")
4	UI after click: resetButton=false managedSetup=true existingRuntimeFields=false (card title "Codex")
5	stored after: {"codex": {"driver": "codex", "enabled": true, "config": {"setupMode": "managed"}}, "claudeAgent": null}
6	after reload: resetButton=false managedSetup=true existingRuntimeFields=false
7
8	## head 7534b06e14 — M2 (edited managed, disabled)
9	seeded: {"driver": "codex", "enabled": false, "displayName": "Personal", "config": {"setupMode": "managed", "launchArgs": "--verbose"}}
10	UI before click: resetButton=true managedSetup=true (status "Disabled", title "Personal")
11	UI after click: resetButton=false managedSetup=true (status "Not authenticated", title "Codex")
12	stored after: {"codex": {"driver": "codex", "enabled": true, "config": {"setupMode": "managed"}}, "claudeAgent": null}
13
14	## head 7534b06e14 — E1 (existing-mode Codex, control)
15	seeded: {"driver": "codex", "enabled": true, "displayName": "Existing proof", "config": {"setupMode": "existing"}}
16	UI before click: resetButton=true existingRuntimeFields=true (title "Existing proof")
17	UI after click: resetButton=false existingRuntimeFields=true (title "Codex")
18	stored after: {"codex": null, "claudeAgent": null}
19
20	## head 7534b06e14 — O1 (Claude default, control)
21	seeded: {"driver": "claudeAgent", "enabled": true, "displayName": "Claude proof"}
22	UI before click: resetButton=true (title "Claude proof")
23	UI after click: resetButton=false (title "Claude")
24	stored after: {"codex": null, "claudeAgent": null}
25
26	## head 7534b06e14 — M0 (raw onboarding write, config.enabled form)
27	seeded: {"driver": "codex", "config": {"enabled": true, "setupMode": "managed"}}
28	UI: resetButton=false managedSetup=true

Not checked: before/after screenshots are not attached (the capture upload tool was unavailable on the verifying machine); the transcript above records the same states. Desktop shares this web panel and was not run separately; mobile has no provider reset.

Claude Opus 5.5 (lead, tests, implementation) and GPT-6.1 Sol (review) via T3 Code and Claude Code.

🤖 Generated with Claude Code

Checks

23 successful · 11 skipped

All checks passed

Added 5 commits3d ago

test(web): reproduce managed Codex reset dropping setup mode (#28885)5ab8e68
fix(web): resetting managed Codex keeps managed setup6b4451b
fix(web): managed Codex reset baseline is the stored onboarding shape7534b06
Merge remote-tracking branch 'origin/main' into fix/issue-28885909ad8b
fix(web): managed Codex reset baseline matches the onboarding writeef74060

MmacroscopeappBotapproved these changes3d ago

MmacroscopeappBot3d ago

Approvability

Verdict: Approved at 909ad8b

Macroscope's review found this PR approvable — Resetting managed Codex now preserves its managed setup instead of falling back to a CLI on PATH. The change is limited to explicit settings resets and leaves other providers’ reset behavior unchanged.

^{You can add or adjust custom eligibility rules. Learn more.}

CcoderabbitaiBot3d ago

Review in Change Stack →

Walkthrough

The settings panel now resets managed Codex instances to their managed configuration baseline. An unchanged managed instance is not marked dirty. Resetting an instance without a managed baseline removes it.

Changes: Managed Codex reset

Layer / File(s)	Summary
Define reset baseline and dirty state `apps/web/src/components/settings/ProviderSettingsPanel.logic.ts`, `apps/web/src/components/settings/ProviderSettingsPanel.tsx`	A helper returns the baseline for managed Codex. The settings panel compares explicit configuration with that baseline to determine whether the instance is dirty.
Persist reset result and validate behavior `apps/web/src/components/settings/ProviderSettingsPanel.tsx`, `apps/web/src/components/settings/ProviderSettingsPanel.environment.browser.test.tsx`	Reset persists the managed baseline when one exists; otherwise, it removes the instance. Browser tests cover edited managed instances, unchanged managed instances, and existing-CLI instances.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested labels: bugfix, size:M

Suggested reviewers: t3dotgg

Merge Risk: External image Minimal · up to 909ad

Managed reset preserves the managed setup and credentials. No material merge risk is established.

Pre-merge checks: ^Passed 4

Check name	Status	Explanation
Title check	Passed	The title clearly and concisely describes the main change: preserving managed Codex setup when resetting provider settings.
Description check	Passed	The description is complete and follows the required sections. It explains the problem, implementation, scope, focused tests, broader checks, runtime verification, and known verification limits.
Linked Issues check	Passed	The changes satisfy the coding requirements in [`#28885`]. `defaultInstanceResetBaseline` recognizes managed Codex instances and returns the stored onboarding baseline. The reset action upserts that bas…
Out of Scope Changes check	Passed	The changes stay within [`#28885`]. The helper changes default reset behavior, the panel applies that helper to dirty-state and reset handling, and the browser tests verify managed and existing-mode Cod…

Full details: Linked Issues check

Explanation

The changes satisfy the coding requirements in [#28885]. defaultInstanceResetBaseline recognizes managed Codex instances and returns the stored onboarding baseline. The reset action upserts that baseline instead of removing the instance. Equal.equals prevents the reset button for an untouched baseline instance. Existing-mode Codex and other default instances still use removal. Focused browser tests cover all three behaviors.

Full details: Out of Scope Changes check

Explanation

The changes stay within [#28885]. The helper changes default reset behavior, the panel applies that helper to dirty-state and reset handling, and the browser tests verify managed and existing-mode Codex behavior. No unrelated product behavior or unrelated files changed.

Finishing Touches

Generate docstrings

Commit to this branch
Create a new PR

Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

CcoderabbitaiBotreviewed3d ago

External image Additional comments: 1

apps/web/src/components/settings/ProviderSettingsPanel.environment.browser.test.tsx: 1

402-404: External image Trivial · Maintainability & Code Quality · Quick win
External image Unverified finding Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.
Use the browser assertion API for the missing reset button.
Replace the synchronous query() assertion with await expect.element(page.getByRole(...)).not.toBeInTheDocument(). (vitest.dev)
As per coding guidelines, “Browser tests assert through expect.element or vi.waitFor.” As per path instructions, “Browser assertions should use expect.element or vi.waitFor.”
Sources: Coding guidelines, Path instructions

External image Review info

External image Run configuration

Source mode: Auto
Repository: pingdotgg/t3code
Branch: fix/issue-28885
File: .coderabbit.config.ts
Commit: 909ad8b
Layers: Repository TS config
Review profile: CHILL
Plan: Team
Run ID: 6de5074d-8cab-4ca9-86f7-d0dbe5c40963

External image Commits

Reviewing files that changed from the base of the PR and between 01a93e7b51a39810c02d87701fa6ab23a2d1bea4 and 909ad8b1678ea4b4aed6b35f449e05d76052f9e9.

External image Files selected for processing: 3

apps/web/src/components/settings/ProviderSettingsPanel.environment.browser.test.tsx
apps/web/src/components/settings/ProviderSettingsPanel.logic.ts
apps/web/src/components/settings/ProviderSettingsPanel.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.