Skip to content

fix(gui): gate Codex pool writes until hydrated and ignore stale strategy polls #739

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Wibias merged 3 commits into from
Jul 30, 2026
Merged

fix(gui): gate Codex pool writes until hydrated and ignore stale strategy polls #739

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c4c98c0
fix(gui): gate Codex pool writes until hydrated and ignore stale stra…
Wibias Jul 30, 2026
fa443c0
fix(gui): address Codex review on pool hydration stack
Wibias Jul 30, 2026
297ceef
fix(gui): address remaining pool hydration review nits
Wibias Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions gui/src/clamp-draft.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/** Clamp a numeric draft string into [min, max] after applying delta (for NumberStepper). */
export function clampNumberDraft(
raw: string,
delta: number,
min: number,
max: number,
step = 1,
): string {
const trimmed = raw.trim();
const parsed = trimmed === "" ? NaN : Number(trimmed);
const base = Number.isFinite(parsed) ? parsed : min;
const next = Math.min(max, Math.max(min, base + delta));
// Keep one decimal for GiB-style steps; integers otherwise.
return step < 1 ? String(Math.round(next * 10) / 10) : String(Math.round(next));
}
8 changes: 8 additions & 0 deletions gui/src/codex-auto-switch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ export function autoSwitchThresholdReadDisposition(
return editing || saving ? "defer" : "apply";
}

/** Accept bare threshold numbers or a full /active payload. */
export function extractAutoSwitchThresholdPayload(value: unknown): unknown {
if (value && typeof value === "object" && value !== null && "autoSwitchThreshold" in value) {
return (value as { autoSwitchThreshold: unknown }).autoSwitchThreshold;
}
return value;
}

export interface AutoSwitchTogglePlan {
threshold: number;
lastEnabled: number;
Expand Down
106 changes: 63 additions & 43 deletions gui/src/components/AccountPoolStrategyControls.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import {
ACCOUNT_POOL_STRATEGIES,
type AccountPoolStrategy,
} from "../account-pool-strategy";
import { clampNumberDraft } from "../clamp-draft";
import { NumberStepper } from "./NumberStepper";
import { Select } from "../ui";

const STRATEGY_LABEL_KEYS = {
quota: "accountPool.strategyQuota",
Expand All @@ -24,7 +27,8 @@ export interface AccountPoolStrategyControlsProps {
strategyLabelHidden?: boolean;
onStrategyChange(strategy: AccountPoolStrategy): void;
onStickyDraftChange(value: string): void;
onStickyCommit(): void;
/** Optional draft overrides React state when steppers commit in the same tick as a draft change. */
onStickyCommit(nextDraft?: string): void;
}

/**
Expand All @@ -42,57 +46,73 @@ export default function AccountPoolStrategyControls({
onStickyCommit,
}: AccountPoolStrategyControlsProps) {
const t = useT();
const strategyOptions = ACCOUNT_POOL_STRATEGIES.map((value) => ({
value,
label: t(STRATEGY_LABEL_KEYS[value]),
}));

return (
<div style={{ marginTop: 12 }}>
<label className="field" style={{ display: "block" }} htmlFor={strategySelectId}>
<span className={strategyLabelHidden ? "sr-only" : "field-label"}>
<div className="account-pool-strategy-controls">
<div className="field">
<span
className={strategyLabelHidden ? "sr-only" : "field-label"}
id={`${strategySelectId}-label`}
>
{t("accountPool.strategy")}
</span>
<select
<Select
id={strategySelectId}

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Forward the strategy select ID through Select

For every GUI build, this passes an unsupported id prop because the existing Select signature has no id field, causing a TypeScript error. Even if transpiled without checking, Select never forwards the ID to its trigger, so the declared label association and ID-based consumers cannot find this control; extend Select to accept and apply the ID or avoid relying on it.

AGENTS.md reference: gui/AGENTS.md:L44-L50

Useful? React with 👍 / 👎.

className="input"
value={strategy}
options={strategyOptions}
disabled={disabled}
aria-label={t("accountPool.strategy")}
onChange={(event) => {
onStrategyChange(event.target.value as AccountPoolStrategy);
}}
>
{ACCOUNT_POOL_STRATEGIES.map((value) => (
<option key={value} value={value}>
{t(STRATEGY_LABEL_KEYS[value])}
</option>
))}
</select>
</label>
<div className="card-sub" style={{ marginTop: 4 }}>
{t("accountPool.strategyHint")}
label={t("accountPool.strategy")}
style={{ width: "100%", display: "block" }}
onChange={(next) => onStrategyChange(next as AccountPoolStrategy)}
/>
</div>
Comment thread
coderabbitai[bot] marked this conversation as resolved.
<div className="card-sub">{t("accountPool.strategyHint")}</div>
{strategy === "round-robin" && (
<label className="field" style={{ display: "block", marginTop: 12 }} htmlFor={stickyInputId}>
<label className="field" htmlFor={stickyInputId}>
<span className="field-label">{t("accountPool.stickyLimit")}</span>
<input
id={stickyInputId}
className="input mono"
type="number"
min={1}
max={100}
step={1}
inputMode="numeric"
value={stickyDraft}
disabled={disabled}
aria-label={t("accountPool.stickyLimitAria")}
onChange={(event) => onStickyDraftChange(event.target.value)}
onBlur={() => onStickyCommit()}
onKeyDown={(event) => {
if (event.nativeEvent.isComposing || disabled) return;
if (event.key === "Enter") {
event.preventDefault();
onStickyCommit();
}
}}
/>
<div className="card-sub" style={{ marginTop: 4 }}>{t("accountPool.stickyLimitHelp")}</div>
<span className="codex-auto-switch-input-wrap">
<input
id={stickyInputId}
className="input mono codex-auto-switch-input"
type="number"
min={1}
max={100}
step={1}
inputMode="numeric"
value={stickyDraft}
disabled={disabled}
aria-label={t("accountPool.stickyLimitAria")}
onChange={(event) => onStickyDraftChange(event.target.value)}
onBlur={() => onStickyCommit()}
onKeyDown={(event) => {
if (event.nativeEvent.isComposing || disabled) return;
if (event.key === "Enter") {
event.preventDefault();
onStickyCommit();
}
}}
/>
<NumberStepper
disabled={disabled}
incrementLabel={t("accountPool.stickyLimitInc")}
decrementLabel={t("accountPool.stickyLimitDec")}
onIncrement={() => {
const next = clampNumberDraft(stickyDraft, 1, 1, 100);
onStickyDraftChange(next);
onStickyCommit(next);
}}
onDecrement={() => {
const next = clampNumberDraft(stickyDraft, -1, 1, 100);
onStickyDraftChange(next);
onStickyCommit(next);
}}
/>
Comment on lines +78 to +113

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Unify the "blocked until hydrated/saving" gating mechanism for numeric stepper inputs. Both files add near-identical numeric input + NumberStepper controls in this PR, but gate interaction differently: one strips the element from the tab order (disabled), the other keeps it focusable but non-editable (readOnly + aria-disabled). This is the same requirement solved two different ways, causing inconsistent keyboard/screen-reader behavior between visually-matching controls.

  • gui/src/components/AccountPoolStrategyControls.tsx#L77-L104: switch the sticky-limit <input> from disabled={disabled} to readOnly={disabled} aria-disabled={disabled} to match the auto-switch threshold input's pattern (or vice versa — pick one convention and apply it to both).
  • gui/src/components/CodexAutoSwitchSetting.tsx#L88-L117: keep as-is if this readOnly+aria-disabled pattern is adopted as the shared convention; otherwise align to whichever mechanism is chosen.
📍 Affects 2 files
  • gui/src/components/AccountPoolStrategyControls.tsx#L77-L104 (this comment)
  • gui/src/components/CodexAutoSwitchSetting.tsx#L88-L117
🤖 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/components/AccountPoolStrategyControls.tsx` around lines 77 - 104,
Unify the blocked-state interaction behavior for the numeric input and
NumberStepper controls. In gui/src/components/AccountPoolStrategyControls.tsx
lines 77-104, replace the sticky-limit input’s disabled gating with the shared
convention used by CodexAutoSwitchSetting.tsx lines 88-117: keep it focusable
with readOnly and aria-disabled when disabled, while leaving
CodexAutoSwitchSetting.tsx unchanged. Ensure both visually matching controls
provide consistent keyboard and screen-reader behavior.

</span>
<div className="card-sub">{t("accountPool.stickyLimitHelp")}</div>
</label>
)}
</div>
Expand Down
Loading
Loading