Skip to content

fix(pool): improve pool management UX — clickable affordance, stale ref handling, managed-by banner#671

Merged
viettranx merged 14 commits intonextlevelbuilder:devfrom
kaitranntt:kai/fix/670-pool-ux
Apr 5, 2026
Merged

fix(pool): improve pool management UX — clickable affordance, stale ref handling, managed-by banner#671
viettranx merged 14 commits intonextlevelbuilder:devfrom
kaitranntt:kai/fix/670-pool-ux

Conversation

@kaitranntt
Copy link
Copy Markdown
Contributor

@kaitranntt kaitranntt commented Apr 3, 2026

Summary

  • Fix pool member selector visibility in dark mode — redesigned buttons with dashed primary border, icon badge, and "Click to add" hint so they're unmistakably interactive in both themes
  • Skip stale pool member references — backend now continues past deleted/disabled providers instead of blocking saves
  • Add managed-by banner for pool members — pool members now show which pool owner manages them with a direct link
  • Add pool discovery badges + setup wizard — per-card "Pool available" badge replaces verbose info banner; one-click wizard to create pools from unpooled accounts
  • Hide pool members from agent provider selector + Pool badge — pool members no longer appear as standalone options in the Provider dropdown; pool owners show a "Pool" badge so users know the provider routes to multiple accounts

Closes #670 · Relates #583

UI Evidence

Full interactive comparison report

1. Pool Owner — Selector Affordance (Dark Mode)

Before After
before after

2. Pool Member — Managed-By Banner

Before After
before after

3. Provider List — Pool Discovery

Before After
before after

4. Agent Provider Selector — Pool Members Hidden + Badge

Before After
before after

5. Pool Setup Wizard (New)

wizard

Test plan

  • Dark mode: pool member selector buttons visible and interactive
  • Light mode: same selector buttons render correctly
  • Save pool config with deleted/disabled member refs — no error
  • Pool member detail page shows managed-by banner with correct owner link
  • Clicking managed-by link navigates via SPA (no full reload)
  • Provider list shows "Pool available" badge on unpooled ChatGPT OAuth accounts (when 2+ exist)
  • Clicking "Pool available" badge opens pool setup wizard
  • Wizard: select owner, members, strategy → Create Pool works
  • Wizard: disabled providers shown but not selectable as members
  • i18n: English, Vietnamese, Chinese translations present for all new strings
  • Agent provider selector hides pool members — verified via Playwright: before shows 4 providers, after shows 3
  • Pool owner shows "Pool" badge in provider dropdown — verified via screenshot (section 4)

viettranx and others added 4 commits April 2, 2026 14:57
nextlevelbuilder#641)

LookupByBinary uses LEFT JOIN with secure_cli_user_credentials but
SELECT columns lacked table alias prefix, causing PostgreSQL error:
"column reference 'id' is ambiguous (SQLSTATE 42702)"

This silently broke ALL credentialed CLI exec — commands fell through
to regular shell exec without injected env vars.

Fix: use b.-prefixed column names for JOIN queries.
Also add diagnostic logging to lookupCredentialedBinary for future debugging.
…evelbuilder#644)

When parallel tool calls trigger loop detection warnings, the warning
messages (role="user") were inserted between tool result messages
(role="tool"). This breaks the Anthropic API when routed through
OpenAI-compatible proxies (e.g. LiteLLM): the proxy groups consecutive
tool messages into a single user message with tool_result blocks, but
an intervening user warning splits the group, causing orphaned
tool_results and HTTP 400 "tool_use ids without tool_result blocks".

Fix: accumulate warning messages during parallel result processing and
append them after all tool results, preserving the consecutive grouping.

Closes nextlevelbuilder#642
nextlevelbuilder#647)

Added ui/web/.npmrc with supportedArchitectures for musl+glibc/arm64+x64.
Updated Dockerfile to use --no-frozen-lockfile so pnpm fetches native rollup
binding compatible with Alpine's musl libc. Lockfile still pinned by copy order.
@kaitranntt kaitranntt marked this pull request as draft April 3, 2026 22:56
kaitranntt added a commit to kaitranntt/goclaw that referenced this pull request Apr 3, 2026
Annotated screenshots with red callout borders marking review areas.
Self-contained HTML comparison report with dark/light theme toggle.
@kaitranntt kaitranntt force-pushed the kai/fix/670-pool-ux branch from 5d772d3 to b7bffe9 Compare April 3, 2026 23:59
@kaitranntt kaitranntt marked this pull request as ready for review April 4, 2026 00:00
Unknown pool member references (deleted or disabled providers) now
continue instead of returning an error. Prevents stale data from
blocking provider saves.

Closes nextlevelbuilder#670
Pool member selector:
- Replace invisible outline button with custom element using dashed
  primary border, + icon badge, and "Click to add" hint text
- Visible in both light and dark themes; hover transitions to solid
  border with shadow; active press scales down for tactile feedback

Managed-by banner:
- Show "Pool Defaults" section on pool members with info banner
  explaining which provider owns the pool, plus a Link navigation
- Previously this section was completely hidden with no explanation

i18n: add poolManagedByDescription and clickToAdd keys (en/vi/zh)
Annotated screenshots with red callout borders marking review areas.
Self-contained HTML comparison report with dark/light theme toggle.
@kaitranntt kaitranntt force-pushed the kai/fix/670-pool-ux branch from b7bffe9 to 6cfba90 Compare April 4, 2026 00:14
@kaitranntt kaitranntt marked this pull request as draft April 4, 2026 01:22
Replace verbose info banner with per-card "Pool available" badge on
unpooled ChatGPT OAuth providers. Clicking the badge opens a new
pool setup wizard dialog where users select owner, members, and
strategy in one step.
@kaitranntt kaitranntt marked this pull request as ready for review April 4, 2026 03:36
@kaitranntt kaitranntt changed the base branch from main to dev April 4, 2026 16:12
- Dockerfile: adopt dev's --frozen-lockfile (lockfile now has musl entries)
- secure_cli.go: take dev's per-agent grant columns, remove duplicate const
- provider-overview.tsx: merge our managed-by banner with dev's formatting,
  remove unused icon imports (Loader2, CheckCircle2, XCircle, AlertTriangle)
Pool member providers are managed via the pool owner's routing config.
Showing them as standalone options in the agent Provider dropdown is
confusing — users may select a member directly instead of the owner,
bypassing pool routing entirely.

Filter out providers that exist in ownerByMember from the enabled
providers list in ProviderModelSelect.
Pool member providers are filtered out of the agent Provider dropdown
in both the Create Agent dialog and the shared ProviderModelSelect
component. Pool owners display a "Pool" badge so users know the
provider routes to multiple accounts automatically.
@mrgoonie
Copy link
Copy Markdown
Contributor

mrgoonie commented Apr 4, 2026

✅ REVIEW COMPLETE — LGTM!

PR #671 — Pool Management UX improvements

Summary

  • 5 fixes (4 UI + 1 backend) — all well-tested with visual evidence
  • CI passing: ✅ go + web checks successful
  • i18n complete: EN/VI/ZH translations present
  • Test plan: Comprehensive, all items verified

Highlights

  1. Selector affordance — Dashed border + icon + "Click to add" label makes it unmistakably interactive (dark mode fixed!)
  2. Stale pool refs — Backend now skips deleted/disabled members instead of blocking saves (pragmatic fix)
  3. Managed-by banner — Pool members now know who owns them with direct SPA link
  4. Pool discovery — Per-card badge is much cleaner than verbose info banner
  5. Wizard — One-click pool setup is a great UX addition

Ready to merge! 🚀

cc: @kaitranntt

- Revert secureCLISelectColsAliased: b.agent_id → b.is_global
  (agent_id was dropped in migration 36, stale merge conflict artifact)
- Replace hardcoded "Pool" badge text with t("providers:list.poolBadge")
  in provider-model-select and agent-identity-and-model-fields
- Replace hardcoded "Disabled" with t("common:disabled") in pool wizard
- Add list.poolBadge key to en/vi/zh locale files
@viettranx
Copy link
Copy Markdown
Contributor

Review fixes pushed — commit 0d90f72

Đã push 1 commit sửa 2 vấn đề tìm thấy trong review:

1. Critical: secure_cli.go — stale merge artifact

  • secureCLISelectColsAliased đổi b.is_globalb.agent_id — nhưng cột agent_id đã bị DROP khỏi bảng secure_cli_binaries trong migration 36 (ngày 4/4)
  • PR branch có vẻ được tạo trước khi merge refactor SecureCLI agent grants
  • Nếu merge, mọi lời gọi LookupByBinary()ListBinariesForAgent() sẽ crash với SQL error column b.agent_id does not exist
  • Fix: Revert về b.is_global (đúng với dev hiện tại)

2. Important: Hardcoded i18n strings

  • "Pool" badge hardcoded ở 2 chỗ (provider-model-select, agent-identity-and-model-fields) → thay bằng t("providers:list.poolBadge")
  • "Disabled" hardcoded trong pool wizard ProviderPlanBadge → thay bằng t("common:disabled")
  • Thêm key list.poolBadge vào cả 3 locale files (en/vi/zh)

Remaining suggestions (không blocking merge):

  • README "Star History" nên tách PR riêng (unrelated to pool UX)
  • Stale member names vẫn tồn tại trong config sau validation skip — xem xét cleanup khi save

@viettranx viettranx merged commit e9733e0 into nextlevelbuilder:dev Apr 5, 2026
2 checks passed
@viettranx
Copy link
Copy Markdown
Contributor

Follow-up: stale member cleanup — commit b65b23b

Đã push thêm 1 commit lên dev để dọn stale member names khi save:

Vấn đề: Khi pool member bị xóa/disabled, tên vẫn nằm trong extra_provider_names của owner — validation skip nhưng rác tích tụ trong DB.

Fix: Thêm stripStalePoolMembers() — chạy sau validation pass, loại member names không tồn tại trong active provider set trước khi ghi DB. Dùng generic map[string]any parse để preserve các settings fields khác ngoài codex_pool.

File: internal/http/chatgpt_oauth_pool_validation.go

@kaitranntt kaitranntt deleted the kai/fix/670-pool-ux branch April 5, 2026 05:55
lecong pushed a commit to lecong/goclaw that referenced this pull request Apr 7, 2026
…ef handling, managed-by banner (nextlevelbuilder#671)

* fix(secure-cli): resolve ambiguous column in LookupByBinary JOIN query (nextlevelbuilder#641)

LookupByBinary uses LEFT JOIN with secure_cli_user_credentials but
SELECT columns lacked table alias prefix, causing PostgreSQL error:
"column reference 'id' is ambiguous (SQLSTATE 42702)"

This silently broke ALL credentialed CLI exec — commands fell through
to regular shell exec without injected env vars.

Fix: use b.-prefixed column names for JOIN queries.
Also add diagnostic logging to lookupCredentialedBinary for future debugging.

* fix(agent): defer warning messages after parallel tool results (nextlevelbuilder#644)

When parallel tool calls trigger loop detection warnings, the warning
messages (role="user") were inserted between tool result messages
(role="tool"). This breaks the Anthropic API when routed through
OpenAI-compatible proxies (e.g. LiteLLM): the proxy groups consecutive
tool messages into a single user message with tool_result blocks, but
an intervening user warning splits the group, causing orphaned
tool_results and HTTP 400 "tool_use ids without tool_result blocks".

Fix: accumulate warning messages during parallel result processing and
append them after all tool results, preserving the consecutive grouping.

Closes nextlevelbuilder#642

* fix(docker): resolve @rollup/rollup-linux-arm64-musl missing on Alpine (nextlevelbuilder#647)

Added ui/web/.npmrc with supportedArchitectures for musl+glibc/arm64+x64.
Updated Dockerfile to use --no-frozen-lockfile so pnpm fetches native rollup
binding compatible with Alpine's musl libc. Lockfile still pinned by copy order.

* docs(README): add history stars (nextlevelbuilder#462)

* fix(pool): skip stale pool member references during validation

Unknown pool member references (deleted or disabled providers) now
continue instead of returning an error. Prevents stale data from
blocking provider saves.

Closes nextlevelbuilder#670

* fix(ui): redesign pool member selector and add managed-by banner

Pool member selector:
- Replace invisible outline button with custom element using dashed
  primary border, + icon badge, and "Click to add" hint text
- Visible in both light and dark themes; hover transitions to solid
  border with shadow; active press scales down for tactile feedback

Managed-by banner:
- Show "Pool Defaults" section on pool members with info banner
  explaining which provider owns the pool, plus a Link navigation
- Previously this section was completely hidden with no explanation

i18n: add poolManagedByDescription and clickToAdd keys (en/vi/zh)

* docs: add before/after UI evidence for PR nextlevelbuilder#671

Annotated screenshots with red callout borders marking review areas.
Self-contained HTML comparison report with dark/light theme toggle.

* feat(ui): add pool discovery badges and setup wizard

Replace verbose info banner with per-card "Pool available" badge on
unpooled ChatGPT OAuth providers. Clicking the badge opens a new
pool setup wizard dialog where users select owner, members, and
strategy in one step.

* docs: update UI evidence with pool discovery before/after

* fix(ui): hide pool members from provider selector in agent forms

Pool member providers are managed via the pool owner's routing config.
Showing them as standalone options in the agent Provider dropdown is
confusing — users may select a member directly instead of the owner,
bypassing pool routing entirely.

Filter out providers that exist in ownerByMember from the enabled
providers list in ProviderModelSelect.

* fix(ui): hide pool members from provider selector and add Pool badge

Pool member providers are filtered out of the agent Provider dropdown
in both the Create Agent dialog and the shared ProviderModelSelect
component. Pool owners display a "Pool" badge so users know the
provider routes to multiple accounts automatically.

* docs: add provider selector before/after evidence

* fix: revert stale merge in secure_cli.go and fix hardcoded i18n strings

- Revert secureCLISelectColsAliased: b.agent_id → b.is_global
  (agent_id was dropped in migration 36, stale merge conflict artifact)
- Replace hardcoded "Pool" badge text with t("providers:list.poolBadge")
  in provider-model-select and agent-identity-and-model-fields
- Replace hardcoded "Disabled" with t("common:disabled") in pool wizard
- Add list.poolBadge key to en/vi/zh locale files

---------

Co-authored-by: Viet Tran <viettranx@gmail.com>
Co-authored-by: Plateau Nguyen <nguyennlt.ncc@gmail.com>
Co-authored-by: DNT <ducconit@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(pool): provider pool management UX confuses users -- stale membership, hidden controls, unclear affordance

5 participants