feat(gui): configure Codex auto-switch threshold#337
Conversation
📝 WalkthroughWalkthroughThe GUI adds configurable Codex account auto-switch thresholds with enable/disable controls, validation, persistence, concurrency-aware loading, localized feedback, responsive styling, tests, and updated dashboard documentation. A separate devlog records root-suite investigation results. ChangesCodex auto-switch threshold
Root-suite investigation record
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CodexAutoSwitchSetting
participant CodexAccountPool
participant useCodexAutoSwitch
participant AutoSwitchEndpoint
User->>CodexAutoSwitchSetting: edit threshold or toggle
CodexAutoSwitchSetting->>useCodexAutoSwitch: commit interaction
useCodexAutoSwitch->>AutoSwitchEndpoint: PUT validated threshold
AutoSwitchEndpoint-->>useCodexAutoSwitch: return response
useCodexAutoSwitch-->>CodexAccountPool: update threshold state
CodexAccountPool-->>CodexAutoSwitchSetting: render feedback and quota threshold
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f84301252
ℹ️ 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".
| onBlur={(event) => { | ||
| if (event.currentTarget.contains(event.relatedTarget as Node | null)) return; | ||
| onEditingChange(false); | ||
| if (enabled && !saving) void onCommit(); |
There was a problem hiding this comment.
When the threshold input is focused and the user clicks the auto-switch toggle after entering a new valid value, the input blur fires before the button click and this handler starts onCommit(), which sets the saving ref/state before the click handler runs. The subsequent toggle click is then ignored or the button is disabled, so the user's single click only saves the draft instead of turning auto-switch off/on; skip the blur commit when focus is moving to the toggle or have the toggle handle the dirty draft in one action.
Useful? React with 👍 / 👎.
5f84301 to
b58c221
Compare
|
Maintainer takeover update: I rebased the contributor commits onto Mounted Root-suite follow-up: the four reported Final verification at Thank you for the original contribution. The contributor branch is updated and green. Because this PR changes |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@devlog/_plan/260724_bugfix_train/061_root_suite_investigation.md`:
- Around line 121-128: Revise the investigation table and the conclusion near
the cited lines so the four success-path tests are labeled only as inferred
candidates, not as the reported failures. State that the original failure names
and assertions remain unresolved at suite level unless CI/test-reporter metadata
is recovered, and remove the unsupported “no fix is justified” closure.
In `@gui/src/codex-auto-switch.ts`:
- Around line 64-80: The putAutoSwitchThreshold request can remain pending
indefinitely because its fetch has no timeout. Add a finite AbortSignal timeout
to the fetch options in putAutoSwitchThreshold, ensuring timeout-triggered
aborts flow through the existing catch and return false while preserving current
validation and response handling.
- Around line 45-62: Update planAutoSwitchToggleWrite so disabling always
returns a plan with threshold 0 when current > 0; use the parsed draft as
lastEnabled only when valid, otherwise fall back to the existing lastEnabled
value. Update the related test assertion to expect a disable write instead of
null for invalid drafts.
In `@gui/src/hooks/useCodexAutoSwitch.ts`:
- Around line 188-202: The toggle disable path must work even when the draft is
unparseable. In gui/src/hooks/useCodexAutoSwitch.ts:188-202, update toggle
around planAutoSwitchToggleWrite to fall back to lastEnabledRef.current when the
plan is null and current > 0, while preserving invalid-draft rejection for other
cases. In gui/tests/codex-auto-switch-controller.test.tsx:177-262, add coverage
that clears the input, clicks .toggle, and asserts a threshold: 0 write without
an invalid-feedback alert.
In `@gui/tests/codex-account-auto-switch.test.tsx`:
- Around line 141-147: Update the read-only attribute assertion in the “keeps
the focused input present but read-only while a write is pending” test to match
renderToStaticMarkup’s lowercase `readonly=""` output, while leaving the other
assertions unchanged.
In `@gui/tests/codex-auto-switch-controller.test.tsx`:
- Around line 177-262: Add regression coverage in the “Codex auto-switch
controller interactions” suite for clicking the toggle with an empty draft:
focus the input, clear it, click the harness toggle, and verify the PUT records
threshold 0 without rendering invalid-feedback alert. Use the existing harness
helpers and preserve the current assertions for other interactions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 761d19a1-597b-474d-8983-fca1f27b9cbb
📒 Files selected for processing (19)
devlog/_plan/260724_bugfix_train/061_root_suite_investigation.mddocs-site/src/content/docs/guides/web-dashboard.mddocs-site/src/content/docs/ja/guides/web-dashboard.mddocs-site/src/content/docs/ko/guides/web-dashboard.mddocs-site/src/content/docs/ru/guides/web-dashboard.mddocs-site/src/content/docs/zh-cn/guides/web-dashboard.mdgui/src/codex-auto-switch.tsgui/src/components/CodexAccountPool.tsxgui/src/components/CodexAutoSwitchSetting.tsxgui/src/hooks/useCodexAutoSwitch.tsgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/src/styles.cssgui/tests/codex-account-auto-switch.test.tsxgui/tests/codex-auto-switch-controller.test.tsx
| - The contributor's retained public report contains only the aggregate `3780 passed, 4 skipped, 4 failed` and says the four failures were in this five-test file. It does not retain the four failing names, assertion text, stderr, or first-failure line. The four rows below map the report's four failure slots to the four success-path cases (the only four cases that require a zero release-helper exit); this mapping is an evidence limitation, not a recovered original log. | ||
|
|
||
| | Reported test name | First failing log/line | Reproduction matrix | Classification | Causal evidence | Owner path | Disposition | | ||
| |---|---|---|---|---|---|---| | ||
| | `preflight runs typecheck, test suite, and privacy scan before version bump on main dry-runs` | Original log unavailable; current source line 216 | PR: standalone 3/3 + full 3/3 pass; base: standalone 1/1 + full 2/2 pass | Non-reproduced (unconfirmed flake) | No PR diff hunk reaches release tooling; every required PR/base order passed | `tests/release-helper.test.ts` | No fix; require final two root runs and exact-head CI | | ||
| | `preview branch still defaults to preview tag and dry-run dispatch` | Original log unavailable; current source line 250 | PR: standalone 3/3 + full 3/3 pass; base: standalone 1/1 + full 2/2 pass | Non-reproduced (unconfirmed flake) | Same behavior and counts on PR and exact base; no repeatable order/resource trigger | `tests/release-helper.test.ts` | No fix; require final two root runs and exact-head CI | | ||
| | `dispatch pins the audited release SHA via expected-sha` | Original log unavailable; current source line 263 | PR: standalone 3/3 + full 3/3 pass; base: standalone 1/1 + full 2/2 pass | Non-reproduced (unconfirmed flake) | Same behavior and counts on PR and exact base; `scripts/release.ts` is unchanged by the PR | `tests/release-helper.test.ts` | No fix; require final two root runs and exact-head CI | | ||
| | `aborts before dispatch when the remote branch moved during the CI wait` | Original log unavailable; current source line 275 | PR: standalone 3/3 + full 3/3 pass; base: standalone 1/1 + full 2/2 pass | Non-reproduced (unconfirmed flake) | Same behavior and counts on PR and exact base; no observed child-process/temp-path leak | `tests/release-helper.test.ts` | No fix; require final two root runs and exact-head CI | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not classify inferred tests as the reported failures.
The document says the original failing names and assertions are unavailable, but then maps the four failure slots to four success-path tests and records each as “Non-reproduced.” That mapping is unsupported: knowing which cases require a zero exit does not prove they were the cases that failed. Consequently, the per-test classifications and Line 136’s “no fix is justified” conclusion cannot be treated as evidence-backed closure.
Recover the original CI/test-reporter metadata, or label these rows as inferred candidates and keep the actual four failures unresolved at suite level.
Also applies to: 136-136
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@devlog/_plan/260724_bugfix_train/061_root_suite_investigation.md` around
lines 121 - 128, Revise the investigation table and the conclusion near the
cited lines so the four success-path tests are labeled only as inferred
candidates, not as the reported failures. State that the original failure names
and assertions remain unresolved at suite level unless CI/test-reporter metadata
is recovered, and remove the unsupported “no fix is justified” closure.
| /** | ||
| * Disabling is one server write. A valid dirty draft becomes the page-lifetime | ||
| * restore value, so re-enabling can persist it without a partial two-write | ||
| * failure state. | ||
| */ | ||
| export function planAutoSwitchToggleWrite( | ||
| current: number, | ||
| draft: string, | ||
| lastEnabled: number, | ||
| ): AutoSwitchTogglePlan | null { | ||
| if (current <= 0) { | ||
| const threshold = nextAutoSwitchThreshold(current, lastEnabled); | ||
| return { threshold, lastEnabled: threshold }; | ||
| } | ||
| const parsedDraft = parseEnabledAutoSwitchThreshold(draft); | ||
| if (parsedDraft === null) return null; | ||
| return { threshold: 0, lastEnabled: parsedDraft }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Disable toggle silently no-ops when the enabled draft is momentarily invalid.
The docstring states "Disabling is one server write," but planAutoSwitchToggleWrite (Lines 59-61) returns null — blocking the write entirely — whenever parseEnabledAutoSwitchThreshold(draft) fails (empty string, decimal, out-of-range, etc.) while current > 0. Since draft tracks the live input rather than the last-committed threshold, a user who clears the field to type a new value (or leaves it briefly invalid) and then clicks the toggle to simply turn the feature off gets no write and, as far as this function is concerned, no feedback — the toggle appears to do nothing. This contradicts the stated "one write" disable guarantee.
Suggest disabling unconditionally and only using the draft opportunistically for the restore value, falling back to the previous lastEnabled when the draft doesn't parse:
🔧 Proposed fix
const parsedDraft = parseEnabledAutoSwitchThreshold(draft);
- if (parsedDraft === null) return null;
- return { threshold: 0, lastEnabled: parsedDraft };
+ return { threshold: 0, lastEnabled: parsedDraft ?? lastEnabled };Note: gui/tests/codex-account-auto-switch.test.tsx Lines 91-94 currently assert null for this exact case and would need updating alongside this fix.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Disabling is one server write. A valid dirty draft becomes the page-lifetime | |
| * restore value, so re-enabling can persist it without a partial two-write | |
| * failure state. | |
| */ | |
| export function planAutoSwitchToggleWrite( | |
| current: number, | |
| draft: string, | |
| lastEnabled: number, | |
| ): AutoSwitchTogglePlan | null { | |
| if (current <= 0) { | |
| const threshold = nextAutoSwitchThreshold(current, lastEnabled); | |
| return { threshold, lastEnabled: threshold }; | |
| } | |
| const parsedDraft = parseEnabledAutoSwitchThreshold(draft); | |
| if (parsedDraft === null) return null; | |
| return { threshold: 0, lastEnabled: parsedDraft }; | |
| } | |
| /** | |
| * Disabling is one server write. A valid dirty draft becomes the page-lifetime | |
| * restore value, so re-enabling can persist it without a partial two-write | |
| * failure state. | |
| */ | |
| export function planAutoSwitchToggleWrite( | |
| current: number, | |
| draft: string, | |
| lastEnabled: number, | |
| ): AutoSwitchTogglePlan | null { | |
| if (current <= 0) { | |
| const threshold = nextAutoSwitchThreshold(current, lastEnabled); | |
| return { threshold, lastEnabled: threshold }; | |
| } | |
| const parsedDraft = parseEnabledAutoSwitchThreshold(draft); | |
| return { threshold: 0, lastEnabled: parsedDraft ?? lastEnabled }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gui/src/codex-auto-switch.ts` around lines 45 - 62, Update
planAutoSwitchToggleWrite so disabling always returns a plan with threshold 0
when current > 0; use the parsed draft as lastEnabled only when valid, otherwise
fall back to the existing lastEnabled value. Update the related test assertion
to expect a disable write instead of null for invalid drafts.
| export async function putAutoSwitchThreshold( | ||
| apiBase: string, | ||
| threshold: number, | ||
| fetchImpl: AutoSwitchFetch = (input, init) => fetch(input, init), | ||
| ): Promise<boolean> { | ||
| if (!Number.isInteger(threshold) || threshold < 0 || threshold > 100) return false; | ||
| try { | ||
| const response = await fetchImpl(`${apiBase}/api/codex-auth/auto-switch`, { | ||
| method: "PUT", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ threshold }), | ||
| }); | ||
| return response.ok; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
No timeout/abort on the auto-switch PUT.
putAutoSwitchThreshold awaits fetchImpl with no AbortSignal/timeout. A stalled network request leaves the "Saving…" state (per CodexAutoSwitchSetting.tsx feedback wiring) pending indefinitely instead of surfacing a timely error. Consider wrapping the fetch with AbortSignal.timeout(...) and treating an abort as a normal false return (already covered by the existing catch).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gui/src/codex-auto-switch.ts` around lines 64 - 80, The
putAutoSwitchThreshold request can remain pending indefinitely because its fetch
has no timeout. Add a finite AbortSignal timeout to the fetch options in
putAutoSwitchThreshold, ensuring timeout-triggered aborts flow through the
existing catch and return false while preserving current validation and response
handling.
| const toggle = useCallback(async (): Promise<boolean> => { | ||
| const current = thresholdRef.current; | ||
| if (current === null || savingRef.current) return false; | ||
| editingRef.current = false; | ||
| const plan = planAutoSwitchToggleWrite(current, draft, lastEnabledRef.current); | ||
| if (plan === null) { | ||
| rejectDraft(); | ||
| return false; | ||
| } | ||
| const ok = await save(plan.threshold, current); | ||
| if (!ok) return false; | ||
| lastEnabledRef.current = plan.lastEnabled; | ||
| if (plan.threshold === 0) setDraftState(String(plan.lastEnabled)); | ||
| return ok; | ||
| }, [draft, rejectDraft, save]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Toggle-to-disable can be blocked by an unparseable draft, and no test catches it. toggle()'s disable path requires parseEnabledAutoSwitchThreshold(draft) to succeed even though disabling doesn't logically need a valid draft, and the test suite never exercises the toggle button to catch this.
gui/src/hooks/useCodexAutoSwitch.ts#L188-L202: fall back tolastEnabledRef.currentwhenplanAutoSwitchToggleWritereturnsnullwhilecurrent > 0, so clicking the toggle always disables regardless of mid-edit draft text (see the standalone comment for the concrete diff).gui/tests/codex-auto-switch-controller.test.tsx#L177-L262: add a test that clears the input then clicks.toggle, asserting athreshold: 0write instead of an invalid-feedback alert, to lock in the fix and prevent regression.
📍 Affects 2 files
gui/src/hooks/useCodexAutoSwitch.ts#L188-L202(this comment)gui/tests/codex-auto-switch-controller.test.tsx#L177-L262
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gui/src/hooks/useCodexAutoSwitch.ts` around lines 188 - 202, The toggle
disable path must work even when the draft is unparseable. In
gui/src/hooks/useCodexAutoSwitch.ts:188-202, update toggle around
planAutoSwitchToggleWrite to fall back to lastEnabledRef.current when the plan
is null and current > 0, while preserving invalid-draft rejection for other
cases. In gui/tests/codex-auto-switch-controller.test.tsx:177-262, add coverage
that clears the input, clicks .toggle, and asserts a threshold: 0 write without
an invalid-feedback alert.
| test("keeps the focused input present but read-only while a write is pending", () => { | ||
| const html = renderSetting(80, "80", true); | ||
| expect(html).toContain('aria-busy="true"'); | ||
| expect(html).toContain('readOnly=""'); | ||
| expect(html).toContain("Saving…"); | ||
| expect(html).toContain('disabled=""'); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the test file and the helper it uses.
git ls-files 'gui/tests/codex-account-auto-switch.test.tsx' 'gui/tests/*' | sed -n '1,20p'
echo
echo "=== outline: gui/tests/codex-account-auto-switch.test.tsx ==="
ast-grep outline gui/tests/codex-account-auto-switch.test.tsx --view expanded || true
echo
echo "=== relevant lines around the assertion ==="
sed -n '120,155p' gui/tests/codex-account-auto-switch.test.tsx
echo
echo "=== search for renderSetting definition/usages ==="
rg -n "function renderSetting|const renderSetting|renderSetting\(" gui/tests gui -g '!**/node_modules/**' || trueRepository: lidge-jun/opencodex
Length of output: 4122
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== file slice: gui/tests/codex-account-auto-switch.test.tsx ==="
sed -n '1,90p' gui/tests/codex-account-auto-switch.test.tsx
echo
echo "=== package.json references for react/rendering libs ==="
rg -n '"react-dom"|renderToStaticMarkup|renderToString|`@testing-library`|jsdom' package.json gui -g '!**/node_modules/**' || trueRepository: lidge-jun/opencodex
Length of output: 4943
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== search for readOnly/readonly expectations in gui/tests ==="
rg -n 'readOnly=""|readonly=""|readOnly\b|readonly\b' gui/tests gui/src -g '!**/node_modules/**' || true
echo
echo "=== inspect similar server-rendered input assertions ==="
sed -n '1,120p' gui/tests/claude-code-autoconnect.test.tsx
echo
sed -n '1,120p' gui/tests/combo-workspace-empty.test.tsx
echo
sed -n '1,120p' gui/tests/models-empty-provider.test.tsxRepository: lidge-jun/opencodex
Length of output: 10378
🌐 Web query:
React renderToStaticMarkup boolean attribute readOnly serialized as readonly
💡 Result:
When using renderToStaticMarkup in React, the readOnly boolean attribute is serialized as readonly="". In HTML, boolean attributes (such as readonly, disabled, or hidden) are valid when they are either present without a value or set to an empty string [1][2][3]. React serializes these attributes with an empty string value (e.g., readonly="") rather than as a standalone attribute (e.g., readonly) [1][3]. Regarding casing, renderToStaticMarkup often preserves the camelCase naming used in JSX (e.g., cellPadding remains cellPadding="" rather than being lowercased to cellpadding="") [4][5]. While HTML attribute names are case-insensitive and browsers correctly interpret them regardless of casing [4][5], this behavior can occasionally cause issues with external tools, email clients, or specific SEO/structured data parsers that expect strictly lowercase HTML attributes [4][6][7]. React maintains this behavior because it does not aim to be a general-purpose HTML renderer and prioritizes performance by avoiding additional processing of attribute names [4][5].
Citations:
- 1: Boolean Attributes on Element Nodes Serialize Differently in MDX mdx-js/mdx#2608
- 2: https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Attributes/readonly
- 3: renderToStaticMarkup incorrectly renders async attribute react/react#11624
- 4: renderToStaticMarkup output has inconsistent attributes case react/react#12815
- 5: React 16 does not lowercase HTML attributes in generated HTML react/react#10863
- 6: Bug:
srcSetprop is rendered tosrcSetattribute (instead ofsrcset) react/react#27344 - 7: Microdata HTML attributes are camelCase in server render react/react#13162
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
try {
const React = require("react");
const { renderToStaticMarkup } = require("react-dom/server");
const html = renderToStaticMarkup(
React.createElement("input", { readOnly: true, disabled: true })
);
console.log(html);
} catch (err) {
console.error(String(err && err.stack || err));
process.exit(1);
}
JSRepository: lidge-jun/opencodex
Length of output: 856
Fix the static-markup assertion for readOnly in gui/tests/codex-account-auto-switch.test.tsx:142-145
renderToStaticMarkup() emits the boolean attribute as readonly="", not readOnly="", so this expectation will miss the rendered HTML and fail. Change the assertion to expect(html).toContain('readonly=""');.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gui/tests/codex-account-auto-switch.test.tsx` around lines 141 - 147, Update
the read-only attribute assertion in the “keeps the focused input present but
read-only while a write is pending” test to match renderToStaticMarkup’s
lowercase `readonly=""` output, while leaving the other assertions unchanged.
| describe("Codex auto-switch controller interactions", () => { | ||
| test("Enter then blur issues exactly one write", async () => { | ||
| const harness = await mountHarness(); | ||
| const write = deferred<Response>(); | ||
| harness.enqueuePut(write.promise); | ||
|
|
||
| await act(async () => { | ||
| harness.input.focus(); | ||
| setInputValue(harness.input, "95"); | ||
| keyDown(harness.input, "Enter"); | ||
| harness.outsideButton.focus(); | ||
| await Promise.resolve(); | ||
| }); | ||
|
|
||
| expect(harness.writes).toEqual([95]); | ||
| await act(async () => { | ||
| write.resolve(new Response(null, { status: 204 })); | ||
| await flush(); | ||
| }); | ||
| expect(harness.input.value).toBe("95"); | ||
| expect(harness.container.querySelector('[role="status"]')?.textContent).toContain("updated"); | ||
| expect(harness.writes).toEqual([95]); | ||
| }); | ||
|
|
||
| test("stale 30-second refresh cannot overwrite a successful edit", async () => { | ||
| const harness = await mountHarness(); | ||
| const staleRead = deferred<Response>(); | ||
| const write = deferred<Response>(); | ||
| harness.enqueueActive(staleRead.promise); | ||
| harness.enqueuePut(write.promise); | ||
|
|
||
| await act(async () => { | ||
| harness.refresh(); | ||
| await Promise.resolve(); | ||
| harness.input.focus(); | ||
| setInputValue(harness.input, "95"); | ||
| keyDown(harness.input, "Enter"); | ||
| await Promise.resolve(); | ||
| }); | ||
| expect(harness.writes).toEqual([95]); | ||
|
|
||
| await act(async () => { | ||
| write.resolve(new Response(null, { status: 204 })); | ||
| await flush(); | ||
| staleRead.resolve(Response.json({ activeCodexAccountId: null, autoSwitchThreshold: 80 })); | ||
| await flush(); | ||
| }); | ||
|
|
||
| expect(harness.input.value).toBe("95"); | ||
| expect(harness.container.textContent).toContain("95% usage or above"); | ||
| expect(harness.writes).toEqual([95]); | ||
| }); | ||
|
|
||
| test("failed write restores the last confirmed value", async () => { | ||
| const harness = await mountHarness(); | ||
| harness.enqueuePut(new Response(null, { status: 500 })); | ||
|
|
||
| await act(async () => { | ||
| harness.input.focus(); | ||
| setInputValue(harness.input, "95"); | ||
| keyDown(harness.input, "Enter"); | ||
| await flush(); | ||
| }); | ||
|
|
||
| expect(harness.input.value).toBe("80"); | ||
| expect(harness.container.textContent).toContain("80% usage or above"); | ||
| expect(harness.container.querySelector('[role="alert"]')?.textContent).toContain("could not be confirmed"); | ||
| expect(harness.writes).toEqual([95]); | ||
| }); | ||
|
|
||
| test("Escape cancels without writing", async () => { | ||
| const harness = await mountHarness(); | ||
|
|
||
| await act(async () => { | ||
| harness.input.focus(); | ||
| setInputValue(harness.input, "95"); | ||
| keyDown(harness.input, "Escape"); | ||
| harness.outsideButton.focus(); | ||
| await flush(); | ||
| }); | ||
|
|
||
| expect(harness.input.value).toBe("80"); | ||
| expect(harness.writes).toEqual([]); | ||
| expect(harness.container.querySelector('[role="status"]')).toBeNull(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
No test coverage for toggle(), including the invalid-draft-blocks-disable bug.
This suite thoroughly covers Enter/blur, stale refresh, failed write, and Escape — but none of the four tests ever clicks the .toggle button, so the bug flagged in gui/src/hooks/useCodexAutoSwitch.ts (Lines 188-202) — where clearing the input and clicking toggle to disable auto-switch surfaces an "invalid" error instead of turning it off — has no regression coverage. Consider adding a test that: focuses the input, clears it (setInputValue(harness.input, "")), then dispatches a click on the toggle button, asserting the PUT is issued with threshold: 0 rather than showing the invalid-feedback alert.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gui/tests/codex-auto-switch-controller.test.tsx` around lines 177 - 262, Add
regression coverage in the “Codex auto-switch controller interactions” suite for
clicking the toggle with an empty draft: focus the input, clear it, click the
harness toggle, and verify the PUT records threshold 0 without rendering
invalid-feedback alert. Use the existing harness helpers and preserve the
current assertions for other interactions.
2 participants
Summary
Verification
bun run typecheck— passed.bun run lint:gui— passed.cd gui && bun run lint:i18n— passed.bun test ./gui/tests— 26 passed, including all 16 threshold cases.bun run privacy:scan— passed.bun run build:gui— passed; existing large-chunk advisory only.cd docs-site && bun run build— passed, 121 pages.bun run doctor:gui:if-changed— advisory exit 0; the only feature-file finding is an existing barrel import.upstream/dev— approved with no findings.The isolated root suite completed with 3780 passed, 4 skipped, and 4 failures confined to the unchanged
release-helpertest.tests/release-helper.test.tsandscripts/release.tshave no diff fromupstream/dev; the complete release-helper file passed 5/5 when rerun standalone.Checklist
Summary by CodeRabbit