feat(providers): Models tab, cap cooldowns, Cursor catalog seeds - #35
Closed
OnlineChef wants to merge 66 commits into
Closed
feat(providers): Models tab, cap cooldowns, Cursor catalog seeds#35OnlineChef wants to merge 66 commits into
OnlineChef wants to merge 66 commits into
Conversation
Init posthog-js only when VITE_POSTHOG_KEY is set; capture hashchange pageviews without identify/PII. Co-authored-by: Cursor <cursoragent@cursor.com>
Greptile P1 on PR #2: window.location.href leaked the full URL (OAuth ?code=, invitation tokens, emails in query/hash) to PostHog as $current_url. Report origin + pathname plus only a known hash route; drop the query string and any unknown hash contents.
OmniRoute (open-source OpenAI-compatible free-model gateway, 250+ providers / 90+ free) wired into the opencodex provider registry as an OpenAI-compatible provider (reuses the existing adapter kind): - src/providers/registry.ts: OmniRoute entry (baseUrl https://api.omniroute.online/v1, configurable via OCX_OMNIROUTE_BASE_URL for self-hosted fleet instances; auth via OCX_OMNIROUTE_KEY), representative free model ids (kimi/glm/deepseek/qwen…). - gui: provider icon + icon mapping so it appears in the provider rail. - tests: provider-registry-parity updated for the new entry. - docs/providers/omniroute.md: setup (key, optional self-hosted Docker), model selection, note that live /v1/models is the source of truth. Additive — no change to existing providers. Local typecheck blocked by a missing bun-types devDep in this worktree (env only; reproduces without this change); CI verifies. Co-authored-by: OnlineChef <280567955+OnlineChef@users.noreply.github.com>
…e-pool max) (#4) * feat(codex): add rotation-mode + request-pacing config types Foundation for multi-account free-pool maximization (opt-in, backward-compat): - codexRotationMode: 'failover' (default, current sticky behavior) | 'round-robin' (new conversations rotate across usable pool accounts so a multi-account free pool multiplies throughput; thread affinity preserved; cooldown/reauth skipped). - codexRequestPacing: jittered per-account inter-request delay [minMs,maxMs], off by default, so a multi-account pool never emits a regular ban-prone pattern. Types only this commit; wiring follows. (Agent run aborted before impl.) * feat(codex): round-robin pool rotation + jittered request pacing Wire the opt-in multi-account free-pool maximization (types in prior commit): - routing.ts: when codexRotationMode === 'round-robin', NEW (non-affined) conversations rotate across getEligiblePoolAccounts via a per-process cursor, so a multi-account free pool multiplies throughput instead of only failing over. Cooldown/reauth/soft-avoid already filtered; thread affinity preserved; activeCodexAccountId untouched; single account = no-op. - pacer.ts: per-account jittered inter-request gap [minMs,maxMs] (defaults 150-900ms), off by default (codexRequestPacing.enabled). Per-account state so concurrent accounts desync instead of a fixed ban-prone cadence. - responses.ts: await codexPaceBeforeSend before the outbound Codex pool send (guarded by usesCodexForwardPoolAuth), no-op when disabled. Tests (bun): 5 pacer + 5 rotation, all pass. typecheck clean. Backward-compat (defaults preserve current failover + no-pacing behavior). --------- Co-authored-by: OnlineChef <280567955+OnlineChef@users.noreply.github.com>
Record upstream, org fork, npm package, and the verified host pin for OnlineChefGroep so ocx upgrades stay explicit. Co-authored-by: Cursor <cursoragent@cursor.com>
Without prompt=select_account an already-signed-in browser re-approves the same JWT sub, so multiauth updates the active row instead of appending. Co-authored-by: Cursor <cursoragent@cursor.com>
… + status endpoint - types.ts: OcxClaudeDesktopProfile에 appliedFingerprint/appliedAt 추가 - desktop-3p.ts: writeDesktop3pConfig에 SHA-256 fingerprint 반환 + metadata write 실패 시 .bak rollback - management-api.ts: /api/claude-desktop/apply에 fingerprint 저장 + /api/claude-desktop/status 엔드포인트 (saved-vs-on-disk 비교)
- claude-messages.ts: Desktop 3P alias 감지 시 logCtx.surface = 'claude-desktop'
- request-log.ts: surface 타입 확장 ('claude' | 'claude-desktop')
- usage/log.ts, usage/summary.ts: surface 필터 확장
- types.ts: desktopAutoApply 옵션 추가 - desktop-health.ts: in-memory 요청/에러 트래커 (NEW) - claude-messages.ts: Desktop 요청 시 recordDesktopRequest() 호출 - management-api.ts: /api/claude-desktop/status에 health 포함 + autoApplyDesktopBestEffort (provider 변경 시 자동 3P config 갱신)
- management-api.ts: buildClaudeDesktopState에 effortSupported 추가 - ClaudeDesktop.tsx: status bar (applied/stale/not-applied) + health (lastRequest, req/err) + effort badge - i18n 4개 언어: status/health/effort 키 추가 - styles.css: .claude-status-bar, .claude-effort-badge 스타일
…arch 260722) - desktop-3p.ts: prefer1m:true 설정 (supports1m 모델에, 공식 스키마 정렬) - desktop-3p.ts: isClaudeShapedId() 가드 추가 (Ollama '0 usable' 거부 방어) - Luna 5레인 연구 기반: 공식 inferenceModels 스키마, 커뮤니티 모델 소실 이슈
- Add nl.ts translations + Dutch pages (Instellingen, Modellen, Systeem, Verkeer) - Refactor App.tsx into provider-workspace shell - Add quota bars + use-provider-quotas hook - Extend de/en i18n with new keys - Add depas.css styling, provider-quota and provider-workspace-shell CSS - Update FLEET.md
… ja/ru Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
- App: re-apply saved theme on mount instead of wiping data-theme - App: route legacy deep links (#codex-auth/#api/#claude/#combos/#subagents) through a per-page sub-target so bookmarks open the right tab/section - App/Modellen/Systeem/Verkeer: drive all visible copy through t() and add the backing keys to every locale dict (en/nl/de/ko/zh/ru/ja) - management-api: return non-OK when a single-provider quota probe yields no report so the GUI can show the refresh-failure state Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…ional divergence Two upstream source-contract tests were merged into this fleet-pin branch but never matched its deliberately divergent implementation, so CI's Test step was red before this change (both already failed at parent f55fcb5). - oauth-tos-warning: this host intentionally drops "cursor" from the elevated ToS-risk set (documented patch in gui/src/oauth-tos-risk.ts) so the warning modal never blocks the multi-account Cursor login flow. Assert cursor is unmarked instead of "elevated"; keep github-copilot as the elevated case. - provider-workspace-rail: App.tsx here localizes the shell to Dutch page ids and models routing as { page, target } via readRouteFromHash/canonicalHash, not the upstream hashBelongsToPage/"providers/workspace" helper. Assert the branch's actual canonicalization + subroute-target threading contract. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…new routing code The GUI lint step was failing on the deep-link/localization work from 05d8801: - react-refresh/only-export-components: move readTheme/applyTheme/Theme out of the Instellingen component file into a dedicated gui/src/theme.ts util module (matching the existing format-bytes.ts / formatUptime.ts convention); import from App + Settings. - react-hooks/set-state-in-effect: replace the target-sync effects in Modellen, Systeem and Verkeer with React's documented render-phase "adjust state when a prop changes" pattern, preserving the deep-link tab/section open behavior. - react-hooks/refs: update pausedRef inside an effect instead of during render in Verkeer. - local-i18n/no-hardcoded-ui-strings: route the remaining receipt-row/detail literals (tok, s, status, upstream:, id) through t() and add vk.rowTokens/rowDuration/detailStatus/ detailUpstream/detailId to all seven locale dicts. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…a dedup, tab reset, routing tests - Instellingen: track theme in React state so the active theme re-renders on select - i18n/shared: keep English browser locales (en-*) instead of defaulting them to Dutch - use-provider-quotas: dedupe in-flight requests by URL so a forced refresh (?refresh=1) never joins/loses to a non-forced one - Modellen: reset the active tab to the default when the routed target is absent/invalid - Extract hash routing into gui/src/route.ts (parseHash/canonicalHash) and rewrite the rail routing test to exercise real parse/canonicalize behavior Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
chore(fleet): reconcile fleet pin with main
docs: document maintainers and review ownership
…low-providers feat(providers): add Tencent and SiliconFlow providers
feat(gui): optional PostHog EU analytics (CHE-785)
… repair, cursor fixes) # Conflicts: # src/server/management-api.ts
# Conflicts: # gui/src/App.tsx # gui/src/i18n/de.ts # gui/src/i18n/en.ts # gui/src/i18n/ja.ts # gui/src/i18n/ko.ts # gui/src/i18n/ru.ts # gui/src/i18n/zh.ts
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+211
to
+223
| if (!response.ok) { | ||
| setActionOk(false); | ||
| setActionStatus(t("models.saveFailed")); | ||
| return; | ||
| } | ||
| setActionOk(true); | ||
| setActionStatus(t("models.applied")); | ||
| setCatalogEpoch(epoch => epoch + 1); | ||
| onRetryModels?.(); | ||
| } catch { | ||
| setActionOk(false); | ||
| setActionStatus(t("models.networkError")); | ||
| } |
There was a problem hiding this comment.
A rejected visibility update leaves the optimistic disabledNamespaced change in place. applyVisibility reports the HTTP or network error but neither restores the prior switch state nor refreshes the catalog, so a model can appear disabled even though the server retained it as enabled.
Artifacts
Focused ProviderModels failure-path test source
- Rendered component harness that starts with the model switch enabled and exercises rejected HTTP PUT and thrown network PUT failures, asserting the optimistic off state remains and no catalog reload occurs.
Focused ProviderModels failure-path test output
- Executed Bun test output from /home/user/repo/gui with exit code 0; records the exact before/after switch state, request sequence, status messages, and both passing failure-path cases.
Prompt To Fix With AI
This is a comment left during a code review.
Path: gui/src/components/provider-workspace/ProviderModels.tsx
Line: 211-223
Comment:
**Failed toggles retain stale state**
A rejected visibility update leaves the optimistic `disabledNamespaced` change in place. `applyVisibility` reports the HTTP or network error but neither restores the prior switch state nor refreshes the catalog, so a model can appear disabled even though the server retained it as enabled.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+246
to
+250
| const bulkToggle = async (enable: boolean) => { | ||
| if (bulkBusy || busyId || fetching || visibleModels.length === 0) return; | ||
| setBulkBusy(true); | ||
| await applyVisibility(visibleModels, enable); | ||
| setBulkBusy(false); |
There was a problem hiding this comment.
The All on/off action submits visibleModels, not the provider's complete model catalog. That list is narrowed by the active search and capped at 300 rendered entries, so filtered-out models and model 301 onward keep their old visibility setting while the UI presents the operation as provider-wide.
Artifacts
Focused ProviderModels bulk-toggle runtime test source
- Authored rendered component test that exercises search-filtered and 301-model All off actions; it captures the outgoing visibility request targets and proves models outside the filtered or 300-item visible subset are omitted.
Focused ProviderModels bulk-toggle runtime test execution output
- Captured Bun test output from /home/user/repo/gui showing both filtered and over-300 request-target assertions passed: 2 pass, 0 fail, exit code 0.
Prompt To Fix With AI
This is a comment left during a code review.
Path: gui/src/components/provider-workspace/ProviderModels.tsx
Line: 246-250
Comment:
**Bulk toggles omit hidden models**
The All on/off action submits `visibleModels`, not the provider's complete model catalog. That list is narrowed by the active search and capped at 300 rendered entries, so filtered-out models and model 301 onward keep their old visibility setting while the UI presents the operation as provider-wide.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
5
to
26
| export type Locale = "en" | "nl"; | ||
| export type { TKey }; | ||
|
|
||
| export const DICTS: Record<Locale, Record<TKey, string>> = { en, nl, de, ko, zh, ru, ja }; | ||
| export const DICTS: Record<Locale, Record<TKey, string>> = { en, nl }; | ||
|
|
||
| export const LOCALES: { code: Locale; name: string; htmlLang: string }[] = [ | ||
| { code: "nl", name: "Nederlands", htmlLang: "nl" }, | ||
| { code: "en", name: "English", htmlLang: "en" }, | ||
| { code: "de", name: "Deutsch", htmlLang: "de" }, | ||
| { code: "ko", name: "한국어", htmlLang: "ko" }, | ||
| { code: "zh", name: "中文", htmlLang: "zh-CN" }, | ||
| { code: "ru", name: "Русский", htmlLang: "ru" }, | ||
| { code: "ja", name: "日本語", htmlLang: "ja" }, | ||
| ]; | ||
|
|
||
| const LANG_KEY = "ocx-lang"; | ||
|
|
||
| export function detectInitial(): Locale { | ||
| try { | ||
| const stored = localStorage.getItem(LANG_KEY); | ||
| if (stored === "en" || stored === "nl" || stored === "de" || stored === "ko" || stored === "zh" || stored === "ru" || stored === "ja") return stored; | ||
| if (stored === "en" || stored === "nl") return stored; | ||
| } catch { /* ignore */ } | ||
| const nav = typeof navigator !== "undefined" ? navigator.language.toLowerCase() : "nl"; | ||
| if (nav.startsWith("en")) return "en"; | ||
| if (nav.startsWith("de")) return "de"; | ||
| if (nav.startsWith("ko")) return "ko"; | ||
| if (nav.startsWith("zh")) return "zh"; | ||
| if (nav.startsWith("ru")) return "ru"; | ||
| if (nav.startsWith("ja")) return "ja"; | ||
| // ChefGroep host build: Joep-facing copy is Dutch by default (DESIGN.md voice-kit). | ||
| // The language switcher in the sidebar still offers English and the rest. | ||
| // The language switcher in the sidebar still offers English. | ||
| return "nl"; |
There was a problem hiding this comment.
Removing de, ja, ko, ru, and zh from the locale registry makes existing stored preferences invalid. Those users fall back to English or Dutch, and the language picker no longer offers their selected language; browsers using those languages also default to Dutch.
Artifacts
Runtime probe source for stored and browser locale detection
- Authored executable probe exercises stored de, ja, ko, ru, zh and browser de-DE, ja-JP, ko-KR, ru-RU, zh-CN against the current locale code; it defines the exact runtime cases.
- Executed `bun trex-artifacts/pr35-locale-runtime-check.ts` from `/home/user/repo` with exit code 0; it shows every removed stored locale falling back to en, every tested non-English browser locale selecting nl, and options nl,en.
Playwright Chromium browser capture source
- Authored Playwright Chromium capture script loads the locale harness with stored de and records observed detection and options; it drives the before and after recordings.
▶ Browser behavior before locale removal
- Playwright Chromium recording of the parent revision with stored de and browser en-US; it shows that the prior locale is retained and remains selectable.
▶ Browser behavior after locale removal
- Playwright Chromium recording of the current revision with stored de and browser en-US; it shows the stored locale is rejected and absent from options.
Prompt To Fix With AI
This is a comment left during a code review.
Path: gui/src/i18n/shared.ts
Line: 5-26
Comment:
**Existing locale preferences are discarded**
Removing `de`, `ja`, `ko`, `ru`, and `zh` from the locale registry makes existing stored preferences invalid. Those users fall back to English or Dutch, and the language picker no longer offers their selected language; browsers using those languages also default to Dutch.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
This reverts commit c389ed4.
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…kspace keys Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…models Persist hard-cap cooldowns (Clinepass INFERENCE_CAP etc.), expose them in /api/config for the Providers UI, and seed kimi-k3 / gemini-3.6-flash / thinking variants in the Cursor static catalog. Co-authored-by: Cursor <cursoragent@cursor.com>
Record/expire weekly inference caps on the startServer config object so routing and /api/config see disables without restart, tighten hard-cap detection, and keep GUI messages short.
OnlineChef
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gui/src/components/provider-workspace/ProviderRail.tsx (1)
76-95: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCapped rows lose their underlying status in the accessible name.
Line 79 replaces
statusentirely witht("pws.capCooldown.badge")("Capped") whenevercappedis true. Thatstatusvalue is the only thing surfaced to non-sighted/non-mouse users (aria-labelat line 95,titleat line 134) — the disabled/needs-attention/needs-setup semantics thatstatusLabel(item, t)normally conveys are silently dropped for a capped-and-disabled or capped-and-needs-setup provider. Sighted users still get both signals (group heading + the separate "Capped" pill at lines 114-116), so this is a real information-loss gap specifically for assistive-tech users.🔧 Proposed fix: combine rather than replace
- const status = capped ? t("pws.capCooldown.badge") : statusLabel(item, t); + const baseStatus = statusLabel(item, t); + const status = capped ? `${baseStatus} · ${t("pws.capCooldown.badge")}` : baseStatus;Also applies to: 134-134
🤖 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/provider-workspace/ProviderRail.tsx` around lines 76 - 95, Update the status construction near statusLabel in the provider rail row so capped providers retain their underlying status while also exposing the cap-cooldown label, combining both labels with the existing accessible separator. Ensure the combined status is used by both the aria-label and title paths, including capped providers that are disabled or need setup.
🤖 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 @.github/workflows/release.yml:
- Around line 120-129: Update the release version checks in the workflow’s
branch-selection logic to enforce the complete channel formats: main releases
must match stable x.y.z semver with no prerelease, and preview releases must
match x.y.z-preview.* without any preceding prerelease identifier. Replace the
broad wildcard checks around RELEASE_VERSION with exact validation consistent
with the contract enforced by the release tooling, and obtain the required
security review for this workflow change.
In `@gui/src/components/provider-workspace/ProviderModels.tsx`:
- Around line 81-92: Update the usageById memoization logic so the unconditional
map.set(key, row) does not overwrite the guarded higher-token selection when key
equals bare. Only set the key separately when it differs from bare, while
preserving the higher-token preference for bare-id collisions and namespaced
lookups.
In `@gui/src/components/provider-workspace/ProviderOverviewDashboard.tsx`:
- Around line 51-78: Update the attention-item construction around
buildAttentionItems and cooldownOverrides so providers present in
cappedProviders are excluded from the generic attention list. Reuse the existing
cap-provider identity data to filter or suppress those entries while preserving
attention items for uncapped providers and the dedicated Usage caps rendering.
In `@gui/src/pages/providers-shared.ts`:
- Around line 18-30: Replace the duplicated ProviderCapCooldown declaration in
the provider configuration types with a shared or generated type derived from
the server-side cap-cooldown definition, preserving the required source field
and all wire-shape fields. Update the providerCooldowns typing to use that
shared type so future server changes cannot silently diverge.
In `@gui/src/styles/provider-workspace-shell.css`:
- Around line 371-392: Replace the deprecated word-break declaration in
.pws-cap-cooldown-msg with the modern overflow-wrap property, preserving the
existing behavior of wrapping long unbreakable cooldown message text.
In `@src/adapters/cursor/discovery.ts`:
- Around line 227-234: Add CURSOR_MODEL_EFFORT_TIERS mappings for every model
marked supportsReasoningEffort in the discovery list, including kimi-k3 and the
newly advertised Claude variants, so cursorEffortSuffix() produces valid tier
suffixes. Update the kimi-model bare-ID exclusion logic in discovery.ts so
kimi-k3 is no longer treated as a model that must remain bare; preserve bare
handling for other applicable Kimi models.
In `@src/providers/cap-cooldown.ts`:
- Around line 87-129: Update recordProviderCapCooldown to mutate the shared live
OcxConfig used by request handling instead of independently loading a fresh
config snapshot via loadConfig. Ensure providerCooldowns and
providers[providerName].disabled are applied to that live reference before
saveConfigPreservingClaudeCode persists it, preserving the existing cooldown and
disable behavior.
In `@tests/cap-cooldown.test.ts`:
- Around line 1-36: Extend the cap-cooldown test suite to cover
recordProviderCapCooldown and expireProviderCooldowns, using the same config
object and load/save path used by production. Assert that recording a cooldown
is visible after round-tripping through
loadConfig/saveConfigPreservingClaudeCode, and verify expiration removes
eligible cooldowns while preserving valid ones.
In `@tests/helpers/shipped-locales.ts`:
- Around line 5-13: The shipped-locale helper must derive SHIPPED_LOCALES from
the runtime DICTS source instead of hard-coding en and nl; update
tests/helpers/shipped-locales.ts lines 5-13 to reuse DICTS while preserving
resolved dictionary values. Update tests/provider-payload.test.ts line 213 to
iterate over the shared SHIPPED_LOCALES helper/map rather than a literal [en,
nl] list.
---
Outside diff comments:
In `@gui/src/components/provider-workspace/ProviderRail.tsx`:
- Around line 76-95: Update the status construction near statusLabel in the
provider rail row so capped providers retain their underlying status while also
exposing the cap-cooldown label, combining both labels with the existing
accessible separator. Ensure the combined status is used by both the aria-label
and title paths, including capped providers that are disabled or need setup.
🪄 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: 8d380c6c-053b-48b7-9e2c-e1f35196f44e
⛔ Files ignored due to path filters (1)
gui/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (43)
.github/workflows/issue-triage.yml.github/workflows/release.ymldocs-site/src/content/docs/contributing.mdgui/src/components/QuotaBars.tsxgui/src/components/provider-workspace/ProviderDetails.tsxgui/src/components/provider-workspace/ProviderModels.tsxgui/src/components/provider-workspace/ProviderOverview.tsxgui/src/components/provider-workspace/ProviderOverviewDashboard.tsxgui/src/components/provider-workspace/ProviderRail.tsxgui/src/components/provider-workspace/ProviderWorkspaceShell.tsxgui/src/formatUptime.tsgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/nl.tsgui/src/i18n/ru.tsgui/src/i18n/shared.tsgui/src/i18n/zh.tsgui/src/pages/Providers.tsxgui/src/pages/providers-shared.tsgui/src/styles/provider-overview-dashboard.cssgui/src/styles/provider-workspace-shell.cssgui/tests/apikeys-layout.test.tsgui/tests/claude-auth-mode-badge.test.tsgui/tests/claude-desktop-locale.test.tsgui/tests/grok-page.test.tsgui/tests/helpers/locales.tsgui/tests/provider-model-custom-add.test.tsxgui/tests/subagents-classic.test.tsgui/tests/usage-grok-filter.test.tsgui/tests/usage-layout.test.tssrc/adapters/cursor/discovery.tssrc/providers/cap-cooldown.tssrc/server/auth-cors.tssrc/server/management/config-routes.tssrc/server/request-log.tstests/cap-cooldown.test.tstests/codex-auth-modal-status.test.tstests/dashboard-uptime.test.tstests/helpers/shipped-locales.tstests/oauth-tos-warning.test.tstests/provider-payload.test.ts
💤 Files with no reviewable changes (5)
- gui/src/i18n/ko.ts
- gui/src/formatUptime.ts
- gui/src/components/QuotaBars.tsx
- gui/src/i18n/zh.ts
- gui/src/i18n/de.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gui/src/components/provider-workspace/ProviderRail.tsx (1)
76-95: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCapped rows lose their underlying status in the accessible name.
Line 79 replaces
statusentirely witht("pws.capCooldown.badge")("Capped") whenevercappedis true. Thatstatusvalue is the only thing surfaced to non-sighted/non-mouse users (aria-labelat line 95,titleat line 134) — the disabled/needs-attention/needs-setup semantics thatstatusLabel(item, t)normally conveys are silently dropped for a capped-and-disabled or capped-and-needs-setup provider. Sighted users still get both signals (group heading + the separate "Capped" pill at lines 114-116), so this is a real information-loss gap specifically for assistive-tech users.🔧 Proposed fix: combine rather than replace
- const status = capped ? t("pws.capCooldown.badge") : statusLabel(item, t); + const baseStatus = statusLabel(item, t); + const status = capped ? `${baseStatus} · ${t("pws.capCooldown.badge")}` : baseStatus;Also applies to: 134-134
🤖 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/provider-workspace/ProviderRail.tsx` around lines 76 - 95, Update the status construction near statusLabel in the provider rail row so capped providers retain their underlying status while also exposing the cap-cooldown label, combining both labels with the existing accessible separator. Ensure the combined status is used by both the aria-label and title paths, including capped providers that are disabled or need setup.
🤖 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 @.github/workflows/release.yml:
- Around line 120-129: Update the release version checks in the workflow’s
branch-selection logic to enforce the complete channel formats: main releases
must match stable x.y.z semver with no prerelease, and preview releases must
match x.y.z-preview.* without any preceding prerelease identifier. Replace the
broad wildcard checks around RELEASE_VERSION with exact validation consistent
with the contract enforced by the release tooling, and obtain the required
security review for this workflow change.
In `@gui/src/components/provider-workspace/ProviderModels.tsx`:
- Around line 81-92: Update the usageById memoization logic so the unconditional
map.set(key, row) does not overwrite the guarded higher-token selection when key
equals bare. Only set the key separately when it differs from bare, while
preserving the higher-token preference for bare-id collisions and namespaced
lookups.
In `@gui/src/components/provider-workspace/ProviderOverviewDashboard.tsx`:
- Around line 51-78: Update the attention-item construction around
buildAttentionItems and cooldownOverrides so providers present in
cappedProviders are excluded from the generic attention list. Reuse the existing
cap-provider identity data to filter or suppress those entries while preserving
attention items for uncapped providers and the dedicated Usage caps rendering.
In `@gui/src/pages/providers-shared.ts`:
- Around line 18-30: Replace the duplicated ProviderCapCooldown declaration in
the provider configuration types with a shared or generated type derived from
the server-side cap-cooldown definition, preserving the required source field
and all wire-shape fields. Update the providerCooldowns typing to use that
shared type so future server changes cannot silently diverge.
In `@gui/src/styles/provider-workspace-shell.css`:
- Around line 371-392: Replace the deprecated word-break declaration in
.pws-cap-cooldown-msg with the modern overflow-wrap property, preserving the
existing behavior of wrapping long unbreakable cooldown message text.
In `@src/adapters/cursor/discovery.ts`:
- Around line 227-234: Add CURSOR_MODEL_EFFORT_TIERS mappings for every model
marked supportsReasoningEffort in the discovery list, including kimi-k3 and the
newly advertised Claude variants, so cursorEffortSuffix() produces valid tier
suffixes. Update the kimi-model bare-ID exclusion logic in discovery.ts so
kimi-k3 is no longer treated as a model that must remain bare; preserve bare
handling for other applicable Kimi models.
In `@src/providers/cap-cooldown.ts`:
- Around line 87-129: Update recordProviderCapCooldown to mutate the shared live
OcxConfig used by request handling instead of independently loading a fresh
config snapshot via loadConfig. Ensure providerCooldowns and
providers[providerName].disabled are applied to that live reference before
saveConfigPreservingClaudeCode persists it, preserving the existing cooldown and
disable behavior.
In `@tests/cap-cooldown.test.ts`:
- Around line 1-36: Extend the cap-cooldown test suite to cover
recordProviderCapCooldown and expireProviderCooldowns, using the same config
object and load/save path used by production. Assert that recording a cooldown
is visible after round-tripping through
loadConfig/saveConfigPreservingClaudeCode, and verify expiration removes
eligible cooldowns while preserving valid ones.
In `@tests/helpers/shipped-locales.ts`:
- Around line 5-13: The shipped-locale helper must derive SHIPPED_LOCALES from
the runtime DICTS source instead of hard-coding en and nl; update
tests/helpers/shipped-locales.ts lines 5-13 to reuse DICTS while preserving
resolved dictionary values. Update tests/provider-payload.test.ts line 213 to
iterate over the shared SHIPPED_LOCALES helper/map rather than a literal [en,
nl] list.
---
Outside diff comments:
In `@gui/src/components/provider-workspace/ProviderRail.tsx`:
- Around line 76-95: Update the status construction near statusLabel in the
provider rail row so capped providers retain their underlying status while also
exposing the cap-cooldown label, combining both labels with the existing
accessible separator. Ensure the combined status is used by both the aria-label
and title paths, including capped providers that are disabled or need setup.
🪄 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: 8d380c6c-053b-48b7-9e2c-e1f35196f44e
⛔ Files ignored due to path filters (1)
gui/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (43)
.github/workflows/issue-triage.yml.github/workflows/release.ymldocs-site/src/content/docs/contributing.mdgui/src/components/QuotaBars.tsxgui/src/components/provider-workspace/ProviderDetails.tsxgui/src/components/provider-workspace/ProviderModels.tsxgui/src/components/provider-workspace/ProviderOverview.tsxgui/src/components/provider-workspace/ProviderOverviewDashboard.tsxgui/src/components/provider-workspace/ProviderRail.tsxgui/src/components/provider-workspace/ProviderWorkspaceShell.tsxgui/src/formatUptime.tsgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/nl.tsgui/src/i18n/ru.tsgui/src/i18n/shared.tsgui/src/i18n/zh.tsgui/src/pages/Providers.tsxgui/src/pages/providers-shared.tsgui/src/styles/provider-overview-dashboard.cssgui/src/styles/provider-workspace-shell.cssgui/tests/apikeys-layout.test.tsgui/tests/claude-auth-mode-badge.test.tsgui/tests/claude-desktop-locale.test.tsgui/tests/grok-page.test.tsgui/tests/helpers/locales.tsgui/tests/provider-model-custom-add.test.tsxgui/tests/subagents-classic.test.tsgui/tests/usage-grok-filter.test.tsgui/tests/usage-layout.test.tssrc/adapters/cursor/discovery.tssrc/providers/cap-cooldown.tssrc/server/auth-cors.tssrc/server/management/config-routes.tssrc/server/request-log.tstests/cap-cooldown.test.tstests/codex-auth-modal-status.test.tstests/dashboard-uptime.test.tstests/helpers/shipped-locales.tstests/oauth-tos-warning.test.tstests/provider-payload.test.ts
💤 Files with no reviewable changes (5)
- gui/src/i18n/ko.ts
- gui/src/formatUptime.ts
- gui/src/components/QuotaBars.tsx
- gui/src/i18n/zh.ts
- gui/src/i18n/de.ts
🛑 Comments failed to post (1)
.github/workflows/release.yml (1)
120-129: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Enforce the complete channel version format.
*-preview.*accepts1.2.3-foo-preview.1, which is valid npm semver but violates thex.y.z-preview.*contract inscripts/release.ts:220-244. A manual dispatch can therefore bypass the release CLI and publish a misclassified preview. Validate both branch formats exactly.Proposed fix
- if [[ "$RELEASE_VERSION" == *-* ]]; then + if [[ ! "$RELEASE_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then echo "::error::main releases must use a stable semver version; got ${RELEASE_VERSION}" exit 1 fi @@ - if [[ "$RELEASE_VERSION" != *-preview.* ]]; then + if [[ ! "$RELEASE_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+-preview\.[[:alnum:]_.]+$ ]]; then echo "::error::preview releases must use a preview prerelease version; got ${RELEASE_VERSION}" exit 1 fiAs per path instructions, workflow changes require explicit security review per MAINTAINERS.md.
📝 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.if [[ ! "$RELEASE_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then echo "::error::main releases must use a stable semver version; got ${RELEASE_VERSION}" exit 1 fi ;; refs/heads/preview) expected_tag="preview" if [[ ! "$RELEASE_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+-preview\.[[:alnum:]_.]+$ ]]; then echo "::error::preview releases must use a preview prerelease version; got ${RELEASE_VERSION}" exit 1🤖 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 @.github/workflows/release.yml around lines 120 - 129, Update the release version checks in the workflow’s branch-selection logic to enforce the complete channel formats: main releases must match stable x.y.z semver with no prerelease, and preview releases must match x.y.z-preview.* without any preceding prerelease identifier. Replace the broad wildcard checks around RELEASE_VERSION with exact validation consistent with the contract enforced by the release tooling, and obtain the required security review for this workflow change.Source: Path instructions
The refreshed Cursor seed marked kimi-k3 and the five claude `-thinking` ids as supportsReasoningEffort without effort-map tiers, so the catalog's bidirectional invariant failed. kimi-k3 gets its verified low/high/max ladder; the `-thinking` ids are complete wire ids with no verified suffix ladder, so they lose the flag and are sent bare like claude-opus-4-7-fast. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Review follow-ups: the bare-id usage map no longer overwrites its own higher-token
pick, capped providers are excluded from the generic attention list now that they
have a dedicated Usage caps section, .pws-cap-cooldown-msg uses overflow-wrap, and
the Cursor seed comment reflects kimi-k3's verified tiers.
The shipped-locale registry moves to a React-free gui/src/i18n/dicts.ts (re-exported
by shared.ts) so tests/helpers/shipped-locales.ts derives from DICTS instead of a
hand-maintained { en, nl } list, and tests/provider-payload.test.ts iterates that
helper.
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…json The cap-cooldown path auto-disables a provider but relied on `GET /api/config` to ever undo it, wrote config.json once per rejected request, and attributed the pause to a display label rather than the routed provider key. - Expiry now runs on a sweep timer bound to the live config, so a headless proxy (the primary mode) re-enables a capped provider at its reset instead of staying disabled until the dashboard is opened or the proxy restarts. Routing reads `providers[].disabled` directly, so this was a hard outage. - An already-active cooldown is authoritative: repeat hard-cap 429s no longer rewrite `until` and re-save. Upstream countdowns are hour-quantized, so only a materially longer window (a 24h cap escalating to weekly) replaces it. - An explicit `disabled` PATCH releases the cooldown's ownership of the flag, so the expiry sweep cannot re-enable a provider an operator turned off. - Cap attribution uses the new `logCtx.providerConfigKey` (the raw routed config key) instead of `logCtx.provider`, which carries a Codex account suffix (`chatgpt-work`) that can collide with a real provider of that name. - `GET /api/config` uses the static imports the module already has. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/server/management/provider-routes.ts (1)
179-189: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRelease cooldown ownership only after the patch is validated.
Line 187 mutates
config.providerCooldowns[name].disabledProvidertofalseimmediately, but the request can still fail after this point. The rejecting branches areproviderManagementConfigError(Line 252),providerDestinationResolvedError(Line 254 and Line 274), and the canonical-openai guard (Line 257). Each returns 400 beforeconfig.providers[name]is assigned at Line 282.Failure mode: the PATCH returns 400 and the provider keeps the
disabled: truethat the cap cooldown set, yet the cooldown entry no longer claims ownership of that flag. When the window ends,expireProviderCooldowns(src/providers/cap-cooldown.tsLines 99-101) deletes the entry without clearingdisabled, becauseentry.disabledProvideris nowfalse. The provider stays disabled after the cap expires, with no cooldown record left to explain it, and the operator must re-enable it by hand. Adisabled-only patch never reaches those branches, but a combined patch such as{ "disabled": false, "baseUrl": "…" }with an invalidbaseUrl, or anopenaire-enable that fails the DNS gate, does.Move the release next to the persisted write so it runs only on the path that actually applies the operator decision.
🐛 Proposed fix — defer the ownership release to the save path
if (Object.hasOwn(rawBody, "disabled")) { if (typeof rawBody.disabled !== "boolean") return jsonResponse({ error: "disabled must be a boolean" }, 400); if (rawBody.disabled && name === config.defaultProvider) { return jsonResponse({ error: "cannot disable the default provider; set another default first" }, 400); } next.disabled = rawBody.disabled; - // An explicit operator decision outranks the cap cooldown: without this the expiry - // sweep would re-enable a provider the operator just turned off by hand. - releaseProviderCooldownDisableOwnership(config, name); touched = true; }Then release it where the change is committed:
const { saveConfigPreservingClaudeCode: save } = await import("../../config"); + // An explicit operator decision outranks the cap cooldown: without this the expiry + // sweep would re-enable a provider the operator just turned off by hand. Run it here, + // after every validation gate, so a rejected patch cannot strip cooldown ownership. + if (Object.hasOwn(rawBody, "disabled")) { + releaseProviderCooldownDisableOwnership(config, name); + } config.providers[name] = stripRegistryOnlyStaticHeaders(name, next); save(config);🤖 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 `@src/server/management/provider-routes.ts` around lines 179 - 189, Move releaseProviderCooldownDisableOwnership from the initial disabled-field handling to the successful persistence path immediately alongside the config.providers[name] assignment. Ensure it executes only after all validation and rejection branches, including providerManagementConfigError, providerDestinationResolvedError, and the canonical-openai guard, have passed; retain the existing disabled-field validation and touched tracking.gui/src/components/provider-workspace/ProviderModels.tsx (3)
457-465: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGive each copy button a model-specific accessible name.
aria-labelreplaces the button's visible model ID in the accessible name. Every copy button therefore exposes the same label, such asCopy model ID. A screen-reader user cannot distinguish which model will be copied.Add localized keys with a
{model}placeholder for both the normal and copied states.Suggested change
- aria-label={copied ? t("pws.modelCopied") : t("pws.copyModelId")} + aria-label={copied + ? t("pws.modelCopiedFor", { model: modelId }) + : t("pws.copyModelIdFor", { model: modelId })}As per coding guidelines, files under
src/componentsmust not hardcode visible UI text; use i18n keys and add each new key to every locale module.🤖 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/provider-workspace/ProviderModels.tsx` around lines 457 - 465, Update the copy button in ProviderModels to use model-specific localized accessible names, passing modelId to new translation keys with a {model} placeholder for both normal and copied states. Add the corresponding keys to every locale module and preserve the existing state-dependent copied versus copy behavior.Source: Coding guidelines
252-259: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSynchronize local state after bulk visibility changes.
bulkToggledoes not update local visibility state.applyVisibilityonly starts an asynchronous/api/modelsreload, and that reload can fail while retaining the old state.bulkBusyis then cleared immediately. The chips can show the old visibility and accept a second action based on stale state after the server accepted the bulk update.Apply the same optimistic update and rollback used for individual changes. Include selected-model state if
modelVisibleuses it.Suggested bulk state transaction
setBulkBusy(true); - await applyVisibility(models, enable); + const previousDisabled = disabledNamespaced; + setDisabledNamespaced(prev => { + const nextSet = new Set(prev); + for (const id of models) { + const namespaced = nativeIds.has(id) ? id : `${item.name}/${id}`; + if (enable) { + nextSet.delete(id); + nextSet.delete(namespaced); + } else { + nextSet.add(namespaced); + } + } + return nextSet; + }); + const saved = await applyVisibility(models, enable); + if (!saved) setDisabledNamespaced(previousDisabled); setBulkBusy(false);As per path instructions, GUI state changes must stay consistent with management API responses.
🤖 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/provider-workspace/ProviderModels.tsx` around lines 252 - 259, Update bulkToggle to use the same optimistic state transaction as individual visibility changes: immediately update local model visibility and any selected-model state used by modelVisible, await applyVisibility, and roll back those updates if the management API operation fails. Keep bulkBusy active until the transaction completes so chips cannot render or accept actions from stale state.Source: Path instructions
193-210: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep local model visibility state consistent with the server call.
ProviderModels.tsx:193-199computes chip state fromselectedModels, whiletoggleModelonly updatesdisabledNamespaced. For non-native models,modelIncludedreturns false whenselectedMap[provider]exists and does not include the model, so enabling an unselected model leaves the switch off untilonRetryModelsrefreshesselectedModels. Add a local selected-model override for this optimistic path, and roll it back whenapplyVisibility(...).okis false.🤖 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/provider-workspace/ProviderModels.tsx` around lines 193 - 210, Update the local visibility state used by isModelOn/modelIncluded so optimistic toggles also reflect selectedModels changes for non-native models. In toggleModel, apply the selected-model override alongside disabledNamespaced before calling applyVisibility, and roll back that override when applyVisibility returns false, preserving the server-refreshed state on success.Source: Path instructions
🤖 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 `@gui/src/i18n/shared.ts`:
- Around line 2-5: Update the locale preference loading and validation flow
associated with Locale and DICTS so saved de, ja, ko, ru, and zh values are
migrated to documented supported locales before runtime validation. Persist each
migrated value, preserve en and nl behavior, and add regression tests covering
every legacy saved preference; do not rely solely on DICTS-derived tests.
In `@src/adapters/cursor/effort-map.ts`:
- Around line 48-49: Add focused regression tests in
tests/cursor-effort-suffix.test.ts for the new "kimi-k3" mapping in
GetUsableModels: assert the exact supported ladder is low, high, and max, and
verify representative Cursor request suffix conversions for those tiers,
including that unsupported medium/xhigh tiers are not accepted.
In `@src/providers/cap-cooldown.ts`:
- Around line 61-70: Update resolveProviderConfigKey to check config.providers
with the repository’s existing own-property helper, hasOwnProvider, instead of
direct truthiness lookup before returning logProvider. Preserve the existing
prefix-matching behavior while ensuring inherited keys such as constructor,
toString, and valueOf cannot resolve as provider configuration keys.
In `@tests/cap-cooldown.test.ts`:
- Around line 127-150: Make the repeat hard-cap test count actual persistence
calls rather than a manual counter increment. Extend
RecordProviderCapCooldownOpts and recordProviderCapCooldown to accept an
optional save callback, invoke it only when a save is genuinely required, and
pass that callback from the test so the 51 records assert exactly one save while
preserving the existing cooldown assertions.
---
Outside diff comments:
In `@gui/src/components/provider-workspace/ProviderModels.tsx`:
- Around line 457-465: Update the copy button in ProviderModels to use
model-specific localized accessible names, passing modelId to new translation
keys with a {model} placeholder for both normal and copied states. Add the
corresponding keys to every locale module and preserve the existing
state-dependent copied versus copy behavior.
- Around line 252-259: Update bulkToggle to use the same optimistic state
transaction as individual visibility changes: immediately update local model
visibility and any selected-model state used by modelVisible, await
applyVisibility, and roll back those updates if the management API operation
fails. Keep bulkBusy active until the transaction completes so chips cannot
render or accept actions from stale state.
- Around line 193-210: Update the local visibility state used by
isModelOn/modelIncluded so optimistic toggles also reflect selectedModels
changes for non-native models. In toggleModel, apply the selected-model override
alongside disabledNamespaced before calling applyVisibility, and roll back that
override when applyVisibility returns false, preserving the server-refreshed
state on success.
In `@src/server/management/provider-routes.ts`:
- Around line 179-189: Move releaseProviderCooldownDisableOwnership from the
initial disabled-field handling to the successful persistence path immediately
alongside the config.providers[name] assignment. Ensure it executes only after
all validation and rejection branches, including providerManagementConfigError,
providerDestinationResolvedError, and the canonical-openai guard, have passed;
retain the existing disabled-field validation and touched tracking.
🪄 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: 23d9bf0a-b4b3-4375-80e9-0ba7c92c7c33
📒 Files selected for processing (19)
gui/AGENTS.mdgui/src/components/provider-workspace/ProviderModels.tsxgui/src/components/provider-workspace/ProviderOverviewDashboard.tsxgui/src/i18n/dicts.tsgui/src/i18n/shared.tsgui/src/styles/provider-workspace-shell.csssrc/adapters/cursor/discovery.tssrc/adapters/cursor/effort-map.tssrc/providers/cap-cooldown.tssrc/server/auth-cors.tssrc/server/index.tssrc/server/management/config-routes.tssrc/server/management/provider-routes.tssrc/server/request-log.tssrc/server/responses/core.tssrc/types.tstests/cap-cooldown.test.tstests/helpers/shipped-locales.tstests/provider-payload.test.ts
Comment on lines
+2
to
+5
| import { DICTS, type Locale, type TKey } from "./dicts"; | ||
|
|
||
| export type Locale = "en" | "de" | "ko" | "zh" | "ru" | "ja"; | ||
| export type { TKey }; | ||
|
|
||
| export const DICTS: Record<Locale, Record<TKey, string>> = { en, de, ko, zh, ru, ja }; | ||
| export type { Locale, TKey }; | ||
| export { DICTS }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Migrate legacy locale preferences before narrowing the runtime locale set.
The runtime now accepts only en and nl. Existing users with saved de, ja, ko, ru, or zh preferences will silently switch to the Dutch fallback after upgrade.
Either retain the previous locale registrations or map each legacy value to a documented supported locale before validation. Persist the migrated value and add regression tests for saved legacy values. The DICTS-derived tests do not cover this path.
The supplied locale change details confirm that validation and detection for the removed locales were deleted.
🤖 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/i18n/shared.ts` around lines 2 - 5, Update the locale preference
loading and validation flow associated with Locale and DICTS so saved de, ja,
ko, ru, and zh values are migrated to documented supported locales before
runtime validation. Persist each migrated value, preserve en and nl behavior,
and add regression tests covering every legacy saved preference; do not rely
solely on DICTS-derived tests.
Comment on lines
+48
to
+49
| // GetUsableModels exposes kimi-k3-{low,high,max} (no medium/xhigh tier). | ||
| "kimi-k3": ["low", "high", "max"], |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add focused regression coverage for the new kimi-k3 suffixes.
The new mapping changes Cursor request construction, but the shown tests/cursor-effort-suffix.test.ts cases cover other ladders only. Add assertions for the exact ladder and representative suffix conversions.
Suggested test coverage
+test("maps kimi-k3 effort tiers to Cursor suffixes", () => {
+ expect(cursorModelEffortLadder("kimi-k3")).toEqual(["low", "high", "max"]);
+ expect(cursorEffortSuffix("kimi-k3", "low")).toBe("low");
+ expect(cursorEffortSuffix("kimi-k3", "max")).toBe("max");
+});As per path instructions, a behavior change in src/ must include focused regression coverage in tests/.
🤖 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 `@src/adapters/cursor/effort-map.ts` around lines 48 - 49, Add focused
regression tests in tests/cursor-effort-suffix.test.ts for the new "kimi-k3"
mapping in GetUsableModels: assert the exact supported ladder is low, high, and
max, and verify representative Cursor request suffix conversions for those
tiers, including that unsupported medium/xhigh tiers are not accepted.
Source: Path instructions
There was a problem hiding this comment.
Already covered: tests/cursor-effort-suffix.test.ts:69-77 asserts cursorModelEffortLadder("kimi-k3") equals ["low","high","max"] plus the low/high/max/bare suffix conversions and that medium clamps to high rather than being accepted literally.
There was a problem hiding this comment.
Skipped: comment is from another GitHub bot.
Comment on lines
+61
to
+70
| /** Map log labels like `openai-work` back to a config.providers key. */ | ||
| export function resolveProviderConfigKey(config: OcxConfig, logProvider: string): string | null { | ||
| if (!logProvider || logProvider === "combo") return null; | ||
| if (config.providers[logProvider]) return logProvider; | ||
| const names = Object.keys(config.providers).sort((a, b) => b.length - a.length); | ||
| for (const name of names) { | ||
| if (logProvider.startsWith(`${name}-`)) return name; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
Use an own-property check in resolveProviderConfigKey.
Line 64 reads config.providers[logProvider] directly. providers is a plain Record, so inherited keys such as constructor, toString, or valueOf return a truthy value. resolveProviderConfigKey then returns that name as a valid config key. In recordProviderCapCooldown the guard on Line 217 is !!config.providers[key], which is also truthy for the same inherited keys, so Line 222 would write disabled = true onto an inherited object instead of a real provider, and Line 233 would persist a providerCooldowns entry for a provider that does not exist.
The repository already uses an own-property helper for this class of key on config.providers (hasOwnProvider in src/server/management/provider-routes.ts Line 132). Apply the same check here so this exported function cannot resolve an inherited key.
🛡️ Proposed hardening
export function resolveProviderConfigKey(config: OcxConfig, logProvider: string): string | null {
if (!logProvider || logProvider === "combo") return null;
- if (config.providers[logProvider]) return logProvider;
+ if (Object.hasOwn(config.providers, logProvider)) return logProvider;
const names = Object.keys(config.providers).sort((a, b) => b.length - a.length);
for (const name of names) {
if (logProvider.startsWith(`${name}-`)) return name;
}
return null;
}📝 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.
Suggested change
| /** Map log labels like `openai-work` back to a config.providers key. */ | |
| export function resolveProviderConfigKey(config: OcxConfig, logProvider: string): string | null { | |
| if (!logProvider || logProvider === "combo") return null; | |
| if (config.providers[logProvider]) return logProvider; | |
| const names = Object.keys(config.providers).sort((a, b) => b.length - a.length); | |
| for (const name of names) { | |
| if (logProvider.startsWith(`${name}-`)) return name; | |
| } | |
| return null; | |
| } | |
| /** Map log labels like `openai-work` back to a config.providers key. */ | |
| export function resolveProviderConfigKey(config: OcxConfig, logProvider: string): string | null { | |
| if (!logProvider || logProvider === "combo") return null; | |
| if (Object.hasOwn(config.providers, logProvider)) return logProvider; | |
| const names = Object.keys(config.providers).sort((a, b) => b.length - a.length); | |
| for (const name of names) { | |
| if (logProvider.startsWith(`${name}-`)) return name; | |
| } | |
| return null; | |
| } |
🤖 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 `@src/providers/cap-cooldown.ts` around lines 61 - 70, Update
resolveProviderConfigKey to check config.providers with the repository’s
existing own-property helper, hasOwnProvider, instead of direct truthiness
lookup before returning logProvider. Preserve the existing prefix-matching
behavior while ensuring inherited keys such as constructor, toString, and
valueOf cannot resolve as provider configuration keys.
Comment on lines
+127
to
+150
| test("repeat hard-cap 429s inside an active window neither rewrite nor re-save", () => { | ||
| const config = bareConfig(); | ||
| const now = Date.UTC(2026, 6, 31, 0, 0, 0); | ||
| let saves = 0; | ||
| const save = () => { saves += 1; }; | ||
| // Stand in for saveConfigPreservingClaudeCode by counting the writes the record path asks for. | ||
| const record = (at: number) => recordProviderCapCooldown(config, "cline-pass", 429, CAP_429, { | ||
| now: at, | ||
| save: false, | ||
| }); | ||
|
|
||
| const first = record(now); | ||
| save(); | ||
| expect(first?.until).toBe(now + (1 * 24 + 22) * HOUR_MS); | ||
|
|
||
| // A retrying client produces one of these per rejected request for the whole cap window. | ||
| for (let i = 1; i <= 50; i += 1) { | ||
| const again = record(now + i * 60_000); | ||
| expect(again?.until).toBe(first?.until); | ||
| expect(again?.recordedAt).toBe(now); | ||
| } | ||
| expect(saves).toBe(1); | ||
| expect(Object.keys(config.providerCooldowns ?? {})).toEqual(["cline-pass"]); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
The saves assertion in this test cannot fail.
Line 133-136 passes save: false to every recordProviderCapCooldown call, so the module never invokes a save. The counter is incremented only by the local save() call at Line 139. The assertion at Line 148, expect(saves).toBe(1), therefore holds no matter what the record path does, and the test name "neither rewrite nor re-save" only verifies the "no rewrite" half through Lines 145-146.
Effect: a future regression that removes the active-cooldown short-circuit at src/providers/cap-cooldown.ts Line 213 would restore one saveConfigPreservingClaudeCode fsync per rejected request for the whole cap window, and this test would still pass.
RecordProviderCapCooldownOpts exposes only save?: boolean, so a real save count needs an injectable saver, the way startProviderCooldownSweep already accepts opts.save. Either extend the options to accept a save callback and count real invocations, or drop the counter so the test does not claim coverage it does not have.
💚 Option A — drop the misleading counter
test("repeat hard-cap 429s inside an active window neither rewrite nor re-save", () => {
const config = bareConfig();
const now = Date.UTC(2026, 6, 31, 0, 0, 0);
- let saves = 0;
- const save = () => { saves += 1; };
- // Stand in for saveConfigPreservingClaudeCode by counting the writes the record path asks for.
const record = (at: number) => recordProviderCapCooldown(config, "cline-pass", 429, CAP_429, {
now: at,
save: false,
});
const first = record(now);
- save();
expect(first?.until).toBe(now + (1 * 24 + 22) * HOUR_MS);
// A retrying client produces one of these per rejected request for the whole cap window.
for (let i = 1; i <= 50; i += 1) {
const again = record(now + i * 60_000);
expect(again?.until).toBe(first?.until);
expect(again?.recordedAt).toBe(now);
}
- expect(saves).toBe(1);
expect(Object.keys(config.providerCooldowns ?? {})).toEqual(["cline-pass"]);
});Option B keeps the stronger guarantee: add an optional saver to RecordProviderCapCooldownOpts (mirroring startProviderCooldownSweep), pass the counter in, and assert exactly one real save across the 51 calls. Do you want me to prepare that change?
🤖 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 `@tests/cap-cooldown.test.ts` around lines 127 - 150, Make the repeat hard-cap
test count actual persistence calls rather than a manual counter increment.
Extend RecordProviderCapCooldownOpts and recordProviderCapCooldown to accept an
optional save callback, invoke it only when a save is genuinely required, and
pass that callback from the test so the 51 records assert exactly one save while
preserving the existing cooldown assertions.
Source: Path instructions
The PR base was retargeted from main to dev, so the branch had to absorb 207 dev commits across 39 conflicted files. Resolutions, by area: - i18n: the branch's en/nl-only registry wins, so de/ja/ko/ru/zh stay deleted. dev's api.workspace.* keys are kept (dev ships the components that use them); the branch's pws.capCooldown.* keys are kept alongside them. The "retired workspace keys stay removed" gates in gui/tests contradicted dev's shipped UI, so they now assert the keys *resolve* in every shipped locale, reading DICTS rather than raw module text (nl spreads en). - Codex routing: dev's accountPoolStrategy engine (round-robin, fill-first, sticky limits, per-scope pools) supersedes the branch's single persisted rrCursor. The fork's codexRotationMode alias would have gone inert, so resolveCodexPoolStrategy() now honours it exactly as dev's own OcxConfig doc promises. tests/rotation-persist.test.ts only pinned the deleted file- persistence mechanism and is removed; the cursor is now process-local, which is dev's documented design. This is a behaviour change: the round-robin cursor no longer survives a restart. - src/usage/*, src/telemetry/posthog-server.ts, src/providers/registry.ts: add/add conflicts where dev is a strict superset (window rolling, sample validation, longest-prefix pricing, the telemetry property allowlist and retry/backoff, the richer OmniRoute entry). Took dev. - types.ts: kept dev's three new OAuth pool configs; dropped dev's duplicate budgets block, which the branch had already added earlier in the interface. - Cursor catalog: union of dev's verified kimi-k3 note and the PR's new *-thinking and gemini-3.6-flash seeds. Also dropped a duplicate kimi-k3 key the auto-merge had silently introduced into effort-map.ts. - release.yml: took dev's. dev's scripts/release.ts pins releaseBranch = "main" and derives the dist-tag from the version, so the branch's main/preview branch-exact guard would have contradicted the script that dispatches it. This touches release automation and needs a maintainer security review. - ci.yml keeps all four jobs (dev's two plus the PR line's dependency audit and actionlint). tests/ci-workflows.test.ts hardcoded a two-job timeout count, so the bounded-job invariant now derives from the parsed job set instead. - gui/package.json keeps both font deps (main.tsx imports jetbrains-mono, depas.css imports ibm-plex-mono) and bun.lock was regenerated. Verified: bun run typecheck, gui tsc -b, lint:gui, lint:i18n, privacy:scan and build:gui all clean. gui suite 405/405. Root suite 6265 pass; the only 3 failures (CLI subcommand help x2, service diagnostics) reproduce on a pristine origin/dev checkout in the same sandbox and are environment-dependent. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…d locales resolveProviderConfigKey read `config.providers[logProvider]` directly, so an inherited member name like `constructor` or `toString` resolved as a provider key: recordProviderCapCooldown would then set `disabled` on Object.prototype's member and persist a providerCooldowns entry for a provider that does not exist. It now uses an own-property check, matching `hasOwnProvider` in src/server/management/provider-routes.ts. The write-amplification test counted its own local save() call, so its `expect(saves).toBe(1)` held regardless of what the record path did and would not have caught a regression in the active-window short-circuit. RecordProviderCapCooldownOpts.save now also accepts a saver function, mirroring startProviderCooldownSweep, and the test injects the counter so the assertion covers all 51 records. detectInitial() dropped saved `de`/`ja`/`ko`/`ru`/`zh` preferences into the Dutch host default. Those users had explicitly chosen a non-Dutch language, so the retired codes now migrate to English and the migrated value is written back. Browser-language detection is unchanged: Dutch stays the default for anything that is not en*. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
The `react-doctor` PR gate (scope `changed`, blocking `warning`) failed with one error and eight warnings on the pages this branch adds to `main`: - `posthog-js@^1.407.0` resolved its range floor to a release Socket scores 46/100 on the supply-chain axis, below the gate's minimum of 50. The lock already resolved 1.409.5 (56/100), so raising the floor to match keeps the installed tree identical and clears the error. - `Instellingen` had an interactive overlay with no role, an unlabeled locale `<select>`, and two `<button>`s defaulting to `type="submit"`. - `Systeem` read `res.json()` before checking status in the update-check, update-run, and sync handlers; they now use the existing `readJsonOrThrow` helper, which preserves the server's error message. - `Verkeer` copied the log array just to sort it; `toSorted` matches the rest of the dashboard. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/ci.yml (3)
148-192: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAudit both dependency lockfiles in the security job.
.github/workflows/ci.ymlinstalls and audits only the rootbun.lock, while the test job also installsgui/bun.lockat lines 77-79. A high-severity finding ingui/bun.lockbypasses this CI security gate. Run the frozen install andbun audit --audit-level=highfor bothbun.lockdirectories, upload both JSON reports, and fail the job when either install/audit exits non-zero.🤖 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 @.github/workflows/ci.yml around lines 148 - 192, Update the security job to audit both the root lockfile and gui/bun.lock: perform frozen installs and high-severity audits from each lockfile’s directory, capture separate JSON reports, and upload both artifacts. Ensure the job records and enforces each install/audit exit status so it fails when either dependency set cannot install or has audit findings, while preserving the existing root audit behavior.Source: Path instructions
194-203: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winTrigger workflow linting for every workflow change.
The
lint-github-actionsjob in.github/workflows/ci.ymlonly runs when the path filters match. Those filters list specific workflow files and omit.github/workflows/issue-triage.yml, so a PR that changes only that workflow can skip the actionlint check.Use
.github/workflows/**in bothpull_request.pathsandpush.paths, or add a separate actionlint workflow with that trigger. Align any path-contract test for workflow changes with the same rule.🤖 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 @.github/workflows/ci.yml around lines 194 - 203, Update the path filters governing the lint-github-actions job so both pull_request.paths and push.paths include the .github/workflows/** pattern, ensuring every workflow change triggers actionlint. Update the related path-contract test to assert the same workflow-wide rule.Source: Path instructions
194-203: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPin the actionlint tool version in
.github/workflows/ci.yml.The pinned reference
raven-actions/actionlint@3d39aea434753780c3b3d4a1a31c854b4dbf49d7 # v2.2.0resolves to a wrapper whoseversioninput defaults tolatest, so a mutablerhysd/actionlintrelease can change this security gate without a workflow diff. Set an approved fixed actionlint version and verify its downloaded binary digest, or use a vendored immutable binary.🤖 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 @.github/workflows/ci.yml around lines 194 - 203, Update the “Lint workflows” step in the lint-github-actions job to stop relying on the wrapper’s mutable latest actionlint version. Configure an approved fixed actionlint version and verify its downloaded binary digest, or replace the wrapper with an immutable vendored actionlint binary while preserving the workflow linting behavior.Sources: Path instructions, MCP tools
🤖 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.
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 148-192: Update the security job to audit both the root lockfile
and gui/bun.lock: perform frozen installs and high-severity audits from each
lockfile’s directory, capture separate JSON reports, and upload both artifacts.
Ensure the job records and enforces each install/audit exit status so it fails
when either dependency set cannot install or has audit findings, while
preserving the existing root audit behavior.
- Around line 194-203: Update the path filters governing the lint-github-actions
job so both pull_request.paths and push.paths include the .github/workflows/**
pattern, ensuring every workflow change triggers actionlint. Update the related
path-contract test to assert the same workflow-wide rule.
- Around line 194-203: Update the “Lint workflows” step in the
lint-github-actions job to stop relying on the wrapper’s mutable latest
actionlint version. Configure an approved fixed actionlint version and verify
its downloaded binary digest, or replace the wrapper with an immutable vendored
actionlint binary while preserving the workflow linting behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0b5a0edb-8404-4212-a4ec-274011e6cd35
📒 Files selected for processing (1)
.github/workflows/ci.yml
5 tasks
Defer cooldown ownership release until after PATCH validation succeeds, sync bulk model toggles with rollback, restore capped-rail status for assistive tech, keep PostHog hashes on the De Pas routes, and audit gui/bun.lock in CI. refs #35
Resolve issue-triage prompt redirect and keep en/nl locale gates. refs #35
The Security audit job now runs `bun audit --audit-level=high` in gui/ as well, and the GUI tree resolved two high-severity transitive advisories: brace-expansion 5.0.6 (GHSA-mh99-v99m-4gvg, GHSA-3jxr-9vmj-r5cp) via minimatch and postcss 8.5.15 (GHSA-r28c-9q8g-f849) via vite. Both are indirect, so pin them through `overrides` (the same mechanism the root package.json already uses for fast-uri) rather than promoting them to direct dependencies. The floors are the patched releases, so the lowest version the range admits is not vulnerable. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Importing posthog.ts in bun tests loaded posthog-js, which crashes happy-dom on EventTarget setup. Move the URL allowlist into a pure module. refs #35
Author
|
Closing without merge. This branch remains a code donor, but the 66-commit cross-cutting diff is no longer a safe review or rollback unit. Reconstruct as separate PRs in this order:
Do not reopen this PR for incremental fixes; cherry-pick or reimplement only coherent pieces into small branches. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PUT /api/model-visibility(same path as the global Models page).POST /api/sync+ reload), with All on/off bulk controls.tokens · requests) from the already-loaded/api/usagerows.Stacked on the en/nl i18n fix branch so
maintypechecks.Test plan
cd gui && bunx tsc -b— 0 errorsNeed help on this PR? Tag
@codesmith-botwith what you need. Autofix is enabled.Greptile Summary
This change adds provider model visibility controls, bulk actions, synchronization, and usage indicators, while limiting the GUI locale list to English and Dutch.
Three reproduced failures make the change unsafe to merge: a rejected single-model update leaves the switch in its optimistic state, bulk actions skip search-hidden and over-cap models, and users with saved German, Japanese, Korean, Russian, or Chinese preferences are silently moved to another language.
Confidence Score: 2/5
What T-Rex did
Comments Outside Diff (1)
General comment
detectInitialreturnsenfor storedde,ja,ko,ru, andzhwhen the browser isen-US; with no stored value it returnsnlforde-DE,ja-JP,ko-KR,ru-RU, andzh-CN.LOCALEScontains onlynl,en.Locale,DICTS, the stored-locale allowlist, the browser-locale branches, and locale options were reduced to English and Dutch ingui/src/i18n/shared.ts:5-26.Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat(gui): provider Models tab toggles, ..." | Re-trigger Greptile
Summary by CodeRabbit