diff --git a/.github/pr-assets/585/after-agent-codex-pool.png b/.github/pr-assets/585/after-agent-codex-pool.png new file mode 100644 index 00000000..f4e0ee41 Binary files /dev/null and b/.github/pr-assets/585/after-agent-codex-pool.png differ diff --git a/.github/pr-assets/585/after-provider-overview.png b/.github/pr-assets/585/after-provider-overview.png new file mode 100644 index 00000000..8fb53626 Binary files /dev/null and b/.github/pr-assets/585/after-provider-overview.png differ diff --git a/.github/pr-assets/585/before-agent-codex-pool.png b/.github/pr-assets/585/before-agent-codex-pool.png new file mode 100644 index 00000000..620457b1 Binary files /dev/null and b/.github/pr-assets/585/before-agent-codex-pool.png differ diff --git a/.github/pr-assets/585/before-provider-overview.png b/.github/pr-assets/585/before-provider-overview.png new file mode 100644 index 00000000..353a6ea8 Binary files /dev/null and b/.github/pr-assets/585/before-provider-overview.png differ diff --git a/.github/pr-assets/585/index.html b/.github/pr-assets/585/index.html new file mode 100644 index 00000000..65f0573a --- /dev/null +++ b/.github/pr-assets/585/index.html @@ -0,0 +1,467 @@ + + + + + + GoClaw Codex Pool UI Before / After + + + +
+
+
UI Test Visual Report
+

Provider-owned Codex pool UI before / after

+

+ This comparison uses two real local GoClaw dashboards seeded with the same fixture data. + The only difference is the code revision: + main@b38a078e versus fix@faf98c73. + The screenshots below isolate the two regressions fixed by the PR: hidden saved pool + membership on provider pages, and sticky custom routing state on agent pages. +

+ +
+
+
Before Revision
+
b38a078e
origin/main
+
+
+
After Revision
+
faf98c73
PR branch
+
+
+
Fixture
+
+ codex-owner owns pool membership
+ codex-disabled is saved as a disabled member
+ solo-agent uses codex-solo with no saved pool defaults +
+
+
+
Capture Method
+
+ Real local UI via Playwright
+ SQLite-backed gateways on separate ports +
+
+
+
+ +
+

Scenario 1: provider page hides saved non-ready members

+

+ The provider page should show saved pool membership even when an extra account is disabled + or not ready. Before the fix, the page derived membership only from quota-fetched ready + accounts, so the saved member disappeared entirely. +

+ +
+
+ What was broken on main +

+ The pool owner codex-owner had a saved extra member + codex-disabled, but the UI rendered 0 in pool, an empty + account list, and no selectable extra account. +

+
+
+ What the patch changes +

+ The page now renders membership from the saved provider settings, while still limiting + quota lookups to ready accounts only. The disabled member remains visible and clearly + labeled as blocked. +

+
+
+ +
+
+
+
Provider Detail
+ Before +
+ Before fix: provider detail hides the saved disabled pool member. +
+ Main branch behavior: +
    +
  • Available Extra Accounts is effectively empty.
  • +
  • Accounts in Pool shows 0 in pool.
  • +
  • The saved disabled member is invisible, so the provider-owned pool looks unsaved.
  • +
+
+
+ +
+
+
Provider Detail
+ After +
+ After fix: provider detail shows the saved disabled pool member and pool count. +
+ Fixed branch behavior: +
    +
  • Codex Disabled is visible in Available Extra Accounts.
  • +
  • Accounts in Pool correctly shows 2 in pool.
  • +
  • The disabled member is still shown as saved, with blocked state clearly surfaced.
  • +
+
+
+
+
+ +
+

Scenario 2: agent page manufactured a sticky custom override

+

+ When an agent had no explicit routing override and the provider had no saved pool yet, + the UI should have represented that state as inherit / use provider defaults. + Before the fix, the draft model incorrectly materialized it as a saved custom + primary_first + [] state. +

+ +
+
+ What was broken on main +

+ The agent page defaulted to Custom For This Agent and framed the agent as + if it had its own routing policy, even though there was no provider-owned pool and no + explicit agent override saved. +

+
+
+ What the patch changes +

+ The draft state now preserves a true inherit mode. The UI shows + Use Provider Defaults and clarifies that the agent stays on its primary + account until provider-owned pool members are later added. +

+
+
+ +
+
+
+
Agent Codex Pool
+ Before +
+ Before fix: agent page defaults to Custom For This Agent even with no provider-owned pool. +
+ Main branch behavior: +
    +
  • Custom For This Agent is selected by default.
  • +
  • The copy tells the user to configure provider members first before enabling pooled routing.
  • +
  • This makes the implicit default state look like an explicit agent override.
  • +
+
+
+ +
+
+
Agent Codex Pool
+ After +
+ After fix: agent page defaults to Use Provider Defaults and explains the temporary primary-only behavior. +
+ Fixed branch behavior: +
    +
  • Use Provider Defaults is selected.
  • +
  • The helper copy explains the temporary primary-only behavior without inventing a custom override.
  • +
  • The agent can stay in inherit mode now and automatically pick up provider pool members later.
  • +
+
+
+
+
+ +
+ Generated from a real local browser session using two SQLite-backed GoClaw instances. + The frontends were served from separate Vite dev servers, authenticated with + system + a local owner token, and seeded through the HTTP API with the same + provider / agent records before capture. +
+
+ + diff --git a/internal/store/agent_store_test.go b/internal/store/agent_store_test.go index 6af59945..1f89cec7 100644 --- a/internal/store/agent_store_test.go +++ b/internal/store/agent_store_test.go @@ -113,6 +113,38 @@ func TestResolveEffectiveChatGPTOAuthRoutingUsesProviderDefaultsWhenAgentUnset(t } } +func TestResolveEffectiveChatGPTOAuthRoutingAllowsInheritWithoutSavedProviderPool(t *testing.T) { + override := &ChatGPTOAuthRoutingConfig{ + OverrideMode: ChatGPTOAuthOverrideInherit, + } + + got := ResolveEffectiveChatGPTOAuthRouting(nil, override) + if got != nil { + t.Fatalf("ResolveEffectiveChatGPTOAuthRouting() = %#v, want nil", got) + } +} + +func TestResolveEffectiveChatGPTOAuthRoutingInheritForwardsProviderDefaults(t *testing.T) { + defaults := &ChatGPTOAuthRoutingConfig{ + Strategy: ChatGPTOAuthStrategyRoundRobin, + ExtraProviderNames: []string{"codex-work"}, + } + override := &ChatGPTOAuthRoutingConfig{ + OverrideMode: ChatGPTOAuthOverrideInherit, + } + + got := ResolveEffectiveChatGPTOAuthRouting(defaults, override) + if got == nil { + t.Fatal("ResolveEffectiveChatGPTOAuthRouting() = nil, want config forwarding provider defaults") + } + if got.Strategy != ChatGPTOAuthStrategyRoundRobin { + t.Fatalf("Strategy = %q, want %q", got.Strategy, ChatGPTOAuthStrategyRoundRobin) + } + if !reflect.DeepEqual(got.ExtraProviderNames, []string{"codex-work"}) { + t.Fatalf("ExtraProviderNames = %#v, want provider defaults", got.ExtraProviderNames) + } +} + func TestResolveEffectiveChatGPTOAuthRoutingAllowsCustomSingleAccountToDisableDefaults(t *testing.T) { defaults := &ChatGPTOAuthRoutingConfig{ Strategy: ChatGPTOAuthStrategyRoundRobin, diff --git a/ui/web/src/i18n/locales/en/agents.json b/ui/web/src/i18n/locales/en/agents.json index df576e76..74cd40df 100644 --- a/ui/web/src/i18n/locales/en/agents.json +++ b/ui/web/src/i18n/locales/en/agents.json @@ -147,7 +147,7 @@ "manageAction": "Open Pool", "singleAccountHint": "Configure another named OpenAI Codex OAuth provider on the Providers page to create a provider-owned pool.", "membershipManagedAtProvider": "Pool members are managed on provider {{provider}}. Change membership there, then come back here to tune routing behavior.", - "membershipConfigureProviderFirst": "No provider-owned pool is saved on {{provider}} yet. Configure pool members there before enabling pooled routing for this agent.", + "membershipConfigureProviderFirst": "No provider-owned pool is saved on {{provider}} yet. This agent will stay on the primary account until pool members are added there.", "readySummary": "{{ready}} of {{total}} extra accounts are ready.", "preferredNeedsAttention": "The primary account needs attention before this agent can use it reliably.", "loadingAccounts": "Checking OpenAI Codex OAuth account status...", @@ -215,7 +215,7 @@ "label": "Agent Routing Mode", "inherit": "Use Provider Defaults", "custom": "Custom For This Agent", - "noProviderDefault": "No provider-owned pool is saved yet. Configure reusable pool members on the Providers page first, or keep this agent on its primary account.", + "noProviderDefault": "No provider-owned pool is saved yet. Following provider defaults will keep this agent on its primary account for now and automatically pick up pool members added later.", "providerDefaultSummary": "{{count}} accounts in provider default", "summaryInherited": "This agent currently follows the provider default pool.", "summaryCustom": "This agent currently saves its own routing override for the provider-owned pool." diff --git a/ui/web/src/i18n/locales/vi/agents.json b/ui/web/src/i18n/locales/vi/agents.json index 39a1158f..61126066 100644 --- a/ui/web/src/i18n/locales/vi/agents.json +++ b/ui/web/src/i18n/locales/vi/agents.json @@ -147,7 +147,7 @@ "manageAction": "Mở Pool", "singleAccountHint": "Hãy cấu hình thêm một provider OpenAI Codex OAuth khác trên trang Providers để tạo pool do provider quản lý.", "membershipManagedAtProvider": "Thành viên pool được quản lý tại provider {{provider}}. Hãy đổi membership ở đó rồi quay lại đây để chỉnh hành vi routing.", - "membershipConfigureProviderFirst": "Provider {{provider}} chưa có provider-owned pool. Hãy cấu hình pool members ở đó trước khi bật pooled routing cho agent này.", + "membershipConfigureProviderFirst": "Provider {{provider}} chưa có provider-owned pool. Agent này sẽ chỉ dùng tài khoản chính cho tới khi pool members được thêm ở đó.", "readySummary": "{{ready}} / {{total}} tài khoản phụ đã sẵn sàng.", "preferredNeedsAttention": "Tài khoản chính cần được xử lý trước khi agent dùng ổn định.", "loadingAccounts": "Đang kiểm tra trạng thái tài khoản OpenAI Codex OAuth...", @@ -215,7 +215,7 @@ "label": "Chế độ routing của agent", "inherit": "Dùng mặc định từ provider", "custom": "Tùy chỉnh riêng cho agent", - "noProviderDefault": "Chưa có provider-owned pool nào được lưu. Hãy cấu hình reusable pool members ở trang Providers trước, hoặc giữ agent này chỉ dùng tài khoản chính.", + "noProviderDefault": "Chưa có provider-owned pool nào được lưu. Chế độ dùng mặc định của provider sẽ giữ agent này ở tài khoản chính lúc này và tự nhận pool members khi provider thêm sau.", "providerDefaultSummary": "Mặc định từ provider có {{count}} tài khoản", "summaryInherited": "Agent này hiện đang dùng pool mặc định của provider.", "summaryCustom": "Agent này hiện đang lưu routing override riêng cho provider-owned pool." diff --git a/ui/web/src/i18n/locales/zh/agents.json b/ui/web/src/i18n/locales/zh/agents.json index 4795e648..37879d0a 100644 --- a/ui/web/src/i18n/locales/zh/agents.json +++ b/ui/web/src/i18n/locales/zh/agents.json @@ -147,7 +147,7 @@ "manageAction": "打开池管理", "singleAccountHint": "请在 Providers 页面配置另一个具名 OpenAI Codex OAuth provider,以创建由 provider 管理的账户池。", "membershipManagedAtProvider": "池成员由 provider {{provider}} 管理。请先在那里修改成员关系,再回到这里调整路由行为。", - "membershipConfigureProviderFirst": "provider {{provider}} 还没有保存 provider-owned pool。请先在那里配置池成员,再为这个 agent 启用池化路由。", + "membershipConfigureProviderFirst": "provider {{provider}} 还没有保存 provider-owned pool。在那里添加池成员之前,这个 agent 会先只使用主账户。", "readySummary": "{{ready}} / {{total}} 个额外账户已就绪。", "preferredNeedsAttention": "主账户需要先处理,agent 才能稳定使用它。", "loadingAccounts": "正在检查 OpenAI Codex OAuth 账户状态...", @@ -215,7 +215,7 @@ "label": "Agent 路由模式", "inherit": "使用 Provider 默认值", "custom": "仅此 Agent 自定义", - "noProviderDefault": "还没有保存 provider-owned pool。请先去 Providers 页面配置可复用的池成员,或者继续让这个 agent 只使用主账户。", + "noProviderDefault": "还没有保存 provider-owned pool。跟随 provider 默认状态时,这个 agent 现在会先停留在主账户,并在以后自动接收新增的池成员。", "providerDefaultSummary": "Provider 默认池包含 {{count}} 个账户", "summaryInherited": "这个 agent 当前正在使用 provider 默认账户池。", "summaryCustom": "这个 agent 当前保存了针对 provider-owned pool 的路由覆盖配置。" diff --git a/ui/web/src/pages/agents/agent-detail/agent-advanced-dialog.tsx b/ui/web/src/pages/agents/agent-detail/agent-advanced-dialog.tsx index b6e3d140..e7bce1fb 100644 --- a/ui/web/src/pages/agents/agent-detail/agent-advanced-dialog.tsx +++ b/ui/web/src/pages/agents/agent-detail/agent-advanced-dialog.tsx @@ -21,6 +21,7 @@ import { buildAgentOtherConfigWithChatGPTOAuthRouting, normalizeChatGPTOAuthRouting, } from "./agent-display-utils"; +import { buildDraftRouting } from "./codex-pool-routing-draft-utils"; interface AgentAdvancedDialogProps { open: boolean; @@ -39,17 +40,10 @@ export function AgentAdvancedDialog({ open, onOpenChange, agent, onUpdate }: Age const deriveState = (a: AgentData) => { const otherObj = (a.other_config ?? {}) as Record; const routing = normalizeChatGPTOAuthRouting(a.other_config); + const draftRouting = buildDraftRouting(routing); return { thinkingLevel: typeof otherObj.thinking_level === "string" ? otherObj.thinking_level : "off", - chatgptRouting: { - override_mode: routing.isExplicit - ? routing.overrideMode - : providerDefaults - ? "inherit" - : "custom", - strategy: routing.strategy, - extra_provider_names: routing.extraProviderNames, - } as ChatGPTOAuthRoutingConfig, + chatgptRouting: draftRouting, wsSharing: (otherObj.workspace_sharing ?? {}) as WorkspaceSharingConfig, comp: a.compaction_config ?? {}, pruneEnabled: a.context_pruning?.mode !== "off", diff --git a/ui/web/src/pages/agents/agent-detail/agent-codex-pool-page.tsx b/ui/web/src/pages/agents/agent-detail/agent-codex-pool-page.tsx index b8828024..ae9bac17 100644 --- a/ui/web/src/pages/agents/agent-detail/agent-codex-pool-page.tsx +++ b/ui/web/src/pages/agents/agent-detail/agent-codex-pool-page.tsx @@ -87,12 +87,12 @@ export function AgentCodexPoolPage() { [agent?.provider, currentProvider?.settings, savedRouting], ); const savedDraftRouting = useMemo( - () => buildDraftRouting(savedRouting, Boolean(providerDefaults)), - [providerDefaults, savedRouting], + () => buildDraftRouting(savedRouting), + [savedRouting], ); const savedDraftSignature = useMemo( - () => routingDraftSignature(savedDraftRouting, Boolean(providerDefaults)), - [providerDefaults, savedDraftRouting], + () => routingDraftSignature(savedDraftRouting), + [savedDraftRouting], ); const [routing, setRouting] = useState(savedDraftRouting); @@ -101,8 +101,8 @@ export function AgentCodexPoolPage() { const savedDraftSignatureRef = useRef(savedDraftSignature); const draftSignature = useMemo( - () => routingDraftSignature(routing, Boolean(providerDefaults)), - [providerDefaults, routing], + () => routingDraftSignature(routing), + [routing], ); useEffect(() => { diff --git a/ui/web/src/pages/agents/agent-detail/codex-pool-routing-draft-utils.ts b/ui/web/src/pages/agents/agent-detail/codex-pool-routing-draft-utils.ts index e13aef94..be00fd0e 100644 --- a/ui/web/src/pages/agents/agent-detail/codex-pool-routing-draft-utils.ts +++ b/ui/web/src/pages/agents/agent-detail/codex-pool-routing-draft-utils.ts @@ -6,7 +6,6 @@ import { export function buildDraftRouting( savedRouting: NormalizedChatGPTOAuthRouting, - hasProviderDefaults: boolean, ): ChatGPTOAuthRoutingConfig { if (savedRouting.isExplicit) { return { @@ -16,16 +15,8 @@ export function buildDraftRouting( }; } - if (hasProviderDefaults) { - return { - override_mode: "inherit", - strategy: "primary_first", - extra_provider_names: [], - }; - } - return { - override_mode: "custom", + override_mode: "inherit", strategy: "primary_first", extra_provider_names: [], }; @@ -33,10 +24,9 @@ export function buildDraftRouting( export function routingDraftSignature( routing: ChatGPTOAuthRoutingConfig, - hasProviderDefaults: boolean, ): string { const normalized = normalizeChatGPTOAuthRoutingInput(routing); - if (normalized.overrideMode === "inherit" && hasProviderDefaults) { + if (normalized.overrideMode === "inherit") { return JSON.stringify({ override_mode: "inherit" }); } return JSON.stringify({ diff --git a/ui/web/src/pages/agents/agent-detail/config-sections/chatgpt-oauth-pool-sections.tsx b/ui/web/src/pages/agents/agent-detail/config-sections/chatgpt-oauth-pool-sections.tsx index 641ba8ed..e5373aa1 100644 --- a/ui/web/src/pages/agents/agent-detail/config-sections/chatgpt-oauth-pool-sections.tsx +++ b/ui/web/src/pages/agents/agent-detail/config-sections/chatgpt-oauth-pool-sections.tsx @@ -43,7 +43,7 @@ interface MembershipSectionProps { membershipManagedByLabel?: string; currentProvider: string; selectedEntries: SelectedEntry[]; - readyExtraProviders: ProviderData[]; + selectableExtraProviders: ProviderData[]; selectedExtras: Set; quotaByName?: Map; canEditMembership: boolean; @@ -57,7 +57,7 @@ export function MembershipSection({ membershipManagedByLabel, currentProvider, selectedEntries, - readyExtraProviders, + selectableExtraProviders, selectedExtras, quotaByName, canEditMembership, @@ -91,9 +91,9 @@ export function MembershipSection({
{t("chatgptOAuthRouting.loadingAccounts")}
- ) : readyExtraProviders.length > 0 ? ( + ) : selectableExtraProviders.length > 0 ? (
- {readyExtraProviders.map((provider) => { + {selectableExtraProviders.map((provider) => { const selected = selectedExtras.has(provider.name); const failureKind = getQuotaFailureKind(quotaByName?.get(provider.name)); return ( diff --git a/ui/web/src/pages/agents/agent-detail/config-sections/chatgpt-oauth-routing-section.tsx b/ui/web/src/pages/agents/agent-detail/config-sections/chatgpt-oauth-routing-section.tsx index 59e232b0..8e14894b 100644 --- a/ui/web/src/pages/agents/agent-detail/config-sections/chatgpt-oauth-routing-section.tsx +++ b/ui/web/src/pages/agents/agent-detail/config-sections/chatgpt-oauth-routing-section.tsx @@ -103,13 +103,15 @@ export function ChatGPTOAuthRoutingSection({ if (!currentOAuthProvider) return null; const allExtraProviders = oauthProviders.filter((p) => p.name !== currentProvider); - const readyExtraProviders = allExtraProviders.filter( - (p) => getAvailabilityFromMap(p, statusByName) === "ready", + const selectedExtras = new Set(value.extra_provider_names ?? []); + const selectableExtraProviders = allExtraProviders.filter( + (p) => + selectedExtras.has(p.name) || + getAvailabilityFromMap(p, statusByName) === "ready", ); const mode = value.override_mode === "inherit" ? "inherit" : "custom"; const providerDefaultsAvailable = defaultRouting != null && defaultRouting.extraProviderNames.length > 0; - const selectedExtras = new Set(value.extra_provider_names ?? []); const selectedEntries = entries.map((entry) => ({ ...entry, @@ -214,7 +216,7 @@ export function ChatGPTOAuthRoutingSection({ type="button" variant={mode === "inherit" ? "default" : "outline"} onClick={() => setMode("inherit")} - disabled={!canManageProviders || !providerDefaultsAvailable} + disabled={!canManageProviders} className="h-9 [@media(max-height:760px)]:h-8" > {t("chatgptOAuthRouting.mode.inherit")} @@ -277,7 +279,7 @@ export function ChatGPTOAuthRoutingSection({ membershipManagedByLabel={membershipManagedByLabel} currentProvider={currentProvider} selectedEntries={selectedEntries} - readyExtraProviders={readyExtraProviders} + selectableExtraProviders={selectableExtraProviders} selectedExtras={selectedExtras} quotaByName={quotaByName} canEditMembership={canEditMembership} diff --git a/ui/web/src/pages/providers/provider-detail/provider-overview.tsx b/ui/web/src/pages/providers/provider-detail/provider-overview.tsx index 616faabe..fdba8d5c 100644 --- a/ui/web/src/pages/providers/provider-detail/provider-overview.tsx +++ b/ui/web/src/pages/providers/provider-detail/provider-overview.tsx @@ -171,6 +171,19 @@ export function ProviderOverview({ provider, onUpdate }: ProviderOverviewProps) strategy: initialRouting?.strategy ?? "primary_first", extra_provider_names: initialRouting?.extraProviderNames ?? [], }); + const selectedPoolProviderNames = useMemo( + () => + canEditPoolRouting + ? Array.from( + new Set( + [provider.name, ...(poolRouting.extra_provider_names ?? [])] + .filter(Boolean) + .filter((name) => name === provider.name || providerByName.has(name)), + ), + ) + : [provider.name], + [canEditPoolRouting, poolRouting.extra_provider_names, provider.name, providerByName], + ); const initEmb = getEmbeddingSettings(provider.settings); const [embEnabled, setEmbEnabled] = useState(initEmb?.enabled ?? false); @@ -253,13 +266,9 @@ export function ProviderOverview({ provider, onUpdate }: ProviderOverviewProps) const quotaProviderNames = useMemo( () => { if (!isOAuth) return []; - const candidateNames = [ - provider.name, - ...(canEditPoolRouting ? poolRouting.extra_provider_names ?? [] : []), - ]; return Array.from( new Set( - candidateNames.filter((providerName) => { + selectedPoolProviderNames.filter((providerName) => { if (!providerName) return false; const item = providerByName.get(providerName); return ( @@ -270,11 +279,9 @@ export function ProviderOverview({ provider, onUpdate }: ProviderOverviewProps) ); }, [ - canEditPoolRouting, isOAuth, - poolRouting.extra_provider_names, - provider.name, providerByName, + selectedPoolProviderNames, statusByName, ], ); @@ -285,7 +292,7 @@ export function ProviderOverview({ provider, onUpdate }: ProviderOverviewProps) } = useChatGPTOAuthProviderQuotas(quotaProviderNames, isOAuth); const poolEntries = useMemo(() => { if (!canEditPoolRouting) return []; - return quotaProviderNames.map((providerName) => { + return selectedPoolProviderNames.map((providerName) => { const item = providerByName.get(providerName); return { name: providerName, @@ -305,10 +312,10 @@ export function ProviderOverview({ provider, onUpdate }: ProviderOverviewProps) quota: quotaByName.get(providerName), }; }); - }, [canEditPoolRouting, provider.name, providerByName, quotaByName, quotaProviderNames, statusByName]); + }, [canEditPoolRouting, provider.name, providerByName, quotaByName, selectedPoolProviderNames, statusByName]); // Provider-scoped pool activity (only for pool owners) - const isPoolOwner = canEditPoolRouting && poolEntries.length > 0; + const isPoolOwner = canEditPoolRouting && selectedPoolProviderNames.length > 1; const { data: poolActivity, isFetching: poolActivityFetching,