diff --git a/docs-site/src/content/docs/guides/providers.md b/docs-site/src/content/docs/guides/providers.md index 3b0cb81edd8..88031b6499e 100644 --- a/docs-site/src/content/docs/guides/providers.md +++ b/docs-site/src/content/docs/guides/providers.md @@ -1417,8 +1417,17 @@ dashboard or `custom` in `ocx init` and enter the base URL. See the Codex asks a second model to review approval requests, and takes that reviewer from `auto_review_model_override` on the catalog row of the current turn's model. The root -`auto_review_model` in `$CODEX_HOME/config.toml` applies one reviewer to every row. To give a -routed provider its own — usually cheaper — reviewer, set the selector on that provider row in +`auto_review_model` in `$CODEX_HOME/config.toml` is Codex's fallback. The dashboard's global +**Auto-review override** in Model settings applies one reviewer to all catalog rows that do not +have a provider-scoped reviewer. It stores `autoReviewOverride` in `~/.opencodex/config.json`; the +setting must be enabled and name a model. An enabled setting with no model stays unapplied and +leaves the Codex fallback in effect. A catalog refresh is a separate state: after a save it can +stay pending, and the Models page reports it so you can retry with `ocx sync`. From the CLI, read +the setting with `ocx models auto-review status` or +change it with `ocx models auto-review set [--enabled ]`; use `-` to clear the +model. CLI output recommends `ocx sync` when a refresh can be retried. If a refresh fails, the +setting remains saved and the command exits 1. Resolve the failure before syncing. To give a routed +provider its own — usually cheaper — reviewer, set the selector on that provider row in `~/.opencodex/config.json`: ```json @@ -1450,9 +1459,10 @@ no per-model override, leaving a valid provider-wide target as fallback. Whateve keep the root selector, or upstream behavior when that is unset. Removing the root selector leaves provider stamps alone, and removing a provider selector clears only that provider's stamps. -These fields are available through configuration, `PATCH /api/providers?name=`, and -the dashboard raw JSON provider editor; dedicated form controls are not present. The canonical `openai` provider -rejects them. Field-by-field rules live in the +Provider fields are available through configuration, `PATCH /api/providers?name=`, and +the dashboard raw JSON provider editor; dedicated provider form controls are not present. The +global override is available in Model settings. The canonical `openai` provider rejects provider +reviewer fields. Field-by-field rules live in the [provider configuration reference](/reference/configuration/providers/#auto-review-approval-model-selection). ## Rate limits in the providers overview diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index 7f6c5792c43..a86c8461001 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -511,9 +511,17 @@ optional pin in a hand-edited file is ignored on load without discarding the res Codex reads `auto_review_model_override` from the catalog row of the current turn's model to choose the model that reviews approval requests. The root `auto_review_model` setting in -`$CODEX_HOME/config.toml` applies one reviewer to task catalog rows; the provider-scoped fields -below override it per provider. The [provider guide](/guides/providers/#approval-reviewer-per-provider) -has the operator workflow and a worked example. +`$CODEX_HOME/config.toml` supplies Codex's fallback. The dashboard's global **Auto-review override** +in Model settings stores `autoReviewOverride` in `~/.opencodex/config.json` and takes precedence +over that fallback; the provider-scoped fields below still win on their own rows. The global setting +must be enabled and name a model. An enabled setting with no model stays unapplied and leaves the +Codex fallback in effect. A catalog refresh is a separate state: after a save it can stay pending, +and the Models page reports it so you can retry with `ocx sync`. CLI users can read the setting with +`ocx models auto-review status` or change it with +`ocx models auto-review set [--enabled ]`; use `-` to clear the model. CLI output +recommends `ocx sync` when a refresh can be retried. If a refresh fails, the setting remains saved +and the command exits 1. Resolve the failure before syncing. The [provider +guide](/guides/providers/#approval-reviewer-per-provider) has the operator workflow and a worked example. When native OpenAI rows are included and the final catalog has an ordinary bare native row other than Reserve (hidden rows count), OpenCodex keeps the hidden `codex-auto-review` row so diff --git a/gui/src/i18n/de.ts b/gui/src/i18n/de.ts index 3abd6ca5ffa..46cac7b3555 100644 --- a/gui/src/i18n/de.ts +++ b/gui/src/i18n/de.ts @@ -480,6 +480,9 @@ export const de: Record = { "dash.shadowCallTooltip": "Die Codex-App ruft im Hintergrund ein Hilfsmodell für Titelgenerierung, Commit-Nachrichten und Skill-Orchestrierung auf. Das Modell wechselt zwischen Client-Versionen, daher fängt opencodex diesen Satz ab: {models}.", "models.shadowCallIntercept": "Shadow-Call-Abfangen", "models.shadowCallInterceptHint": "Fängt die Hintergrund-Hilfsaufrufe der Codex-App ({models}) ab und leitet sie an das gewählte Modell um.", + "models.autoReviewOverride": "Auto-Review-Überschreibung", + "models.autoReviewOverrideHint": "Legt fest, welches Modell Codex für die automatische Prüfung (Approval-Reviewer) verwendet. Wird beim Katalog-Abgleich auf alle Zeilen angewendet; ein autoReviewModel auf Anbieterebene hat weiterhin Vorrang.", + "models.autoReviewPending": "Ausstehend", "dash.sidecarBackend": "Backend", "dash.sidecarModel": "Modell", "dash.backendAuto": "Automatisch", diff --git a/gui/src/i18n/en.ts b/gui/src/i18n/en.ts index 594622bfcb1..9b94dce12ea 100644 --- a/gui/src/i18n/en.ts +++ b/gui/src/i18n/en.ts @@ -498,6 +498,9 @@ export const en = { "dash.shadowCallTooltip": "Codex App makes background helper calls for thread title generation, commit message generation, and skill orchestration. The helper model changed across client versions, so opencodex intercepts every model in this set: {models}. Enable this to redirect those calls to your chosen model.", "models.shadowCallIntercept": "Shadow Call Intercept", "models.shadowCallInterceptHint": "Intercepts Codex App's background helper calls ({models}) for titles and commit messages and redirects them to your chosen model.", + "models.autoReviewOverride": "Auto-review override", + "models.autoReviewOverrideHint": "Chooses the model Codex uses for auto-review (the approval reviewer). Applied to every catalog row at sync; a provider-scoped autoReviewModel still takes precedence on its own rows.", + "models.autoReviewPending": "Pending", "dash.sidecarBackend": "Backend", "dash.sidecarModel": "Model", "dash.backendAuto": "Auto", diff --git a/gui/src/i18n/fr.ts b/gui/src/i18n/fr.ts index d04be08d10b..94ab2f9008c 100644 --- a/gui/src/i18n/fr.ts +++ b/gui/src/i18n/fr.ts @@ -488,6 +488,9 @@ export const fr: Record = { "dash.shadowCallTooltip": "L’application Codex effectue des appels auxiliaires en arrière-plan pour générer les titres de fils, les messages de commit et orchestrer les compétences. Le modèle auxiliaire ayant changé selon les versions du client, opencodex intercepte tous les modèles de cet ensemble : {models}. Activez cette option pour rediriger ces appels vers le modèle choisi.", "models.shadowCallIntercept": "Interception des appels fantômes", "models.shadowCallInterceptHint": "Intercepte les appels auxiliaires en arrière-plan de l’application Codex ({models}) pour les titres et les messages de commit, puis les redirige vers le modèle choisi.", + "models.autoReviewOverride": "Remplacement de la revue automatique", + "models.autoReviewOverrideHint": "Choisit le modèle que Codex utilise pour la revue automatique (le modèle de revue des demandes d'approbation). Appliqué à chaque ligne du catalogue à la synchronisation ; un autoReviewModel défini au niveau du fournisseur reste prioritaire sur ses lignes.", + "models.autoReviewPending": "En attente", "dash.sidecarBackend": "Moteur", "dash.sidecarModel": "Modèle", "dash.backendAuto": "Auto", diff --git a/gui/src/i18n/ja.ts b/gui/src/i18n/ja.ts index cc45d5dc154..646ced80903 100644 --- a/gui/src/i18n/ja.ts +++ b/gui/src/i18n/ja.ts @@ -489,6 +489,9 @@ export const ja: Record = { "dash.shadowCallTooltip": "Codex App はスレッドタイトル生成、コミットメッセージ生成、スキルオーケストレーションをバックグラウンドで呼び出します。使われるモデルはクライアントのバージョンによって変わるため、opencodex は {models} をまとめて傍受します。これをオンにすると、それらの呼び出しを選択したモデルにリダイレクトします。", "models.shadowCallIntercept": "シャドウコール傍受", "models.shadowCallInterceptHint": "Codex App のバックグラウンドヘルパー呼び出し({models}: タイトル、コミットメッセージ)を傍受し、選択したモデルにリダイレクトします。", + "models.autoReviewOverride": "自動レビューの上書き", + "models.autoReviewOverrideHint": "Codex の自動レビュー（承認レビュアー）が使うモデルを指定します。カタログ同期時にすべての行へ適用され、プロバイダー固有の autoReviewModel がその行では引き続き優先されます。", + "models.autoReviewPending": "保留中", "dash.sidecarBackend": "バックエンド", "dash.sidecarModel": "モデル", "dash.backendAuto": "自動", diff --git a/gui/src/i18n/ko.ts b/gui/src/i18n/ko.ts index 0a0e0dc0351..544e19ed0d6 100644 --- a/gui/src/i18n/ko.ts +++ b/gui/src/i18n/ko.ts @@ -484,6 +484,9 @@ export const ko: Record = { "dash.shadowCallTooltip": "Codex 앱은 스레드 제목 자동 생성, 커밋 메시지 생성, 스킬 오케스트레이션 같은 내부 작업을 백그라운드로 호출합니다. 이때 쓰는 모델은 클라이언트 버전마다 달라서 opencodex는 {models}를 모두 가로챕니다. 이 설정을 켜면 해당 호출이 선택한 모델로 넘어갑니다.", "models.shadowCallIntercept": "쉐도우 호출 가로채기", "models.shadowCallInterceptHint": "Codex 앱의 백그라운드 호출({models}, 제목·커밋 메시지)을 가로채 선택한 모델로 바꿉니다.", + "models.autoReviewOverride": "자동 검토 재정의", + "models.autoReviewOverrideHint": "Codex의 자동 검토(승인 검토자)에 사용할 모델을 지정합니다. 카탈로그 동기화 시 모든 행에 적용되며, 공급자별 autoReviewModel이 해당 행에서 계속 우선합니다.", + "models.autoReviewPending": "보류 중", "dash.sidecarBackend": "백엔드", "dash.sidecarModel": "모델", "dash.backendAuto": "자동", diff --git a/gui/src/i18n/pt.ts b/gui/src/i18n/pt.ts index 70b9a906840..0a92303ec3f 100644 --- a/gui/src/i18n/pt.ts +++ b/gui/src/i18n/pt.ts @@ -502,6 +502,9 @@ export const pt: Record = { "dash.shadowCallTooltip": "O Codex App faz chamadas auxiliares em segundo plano para geração de título de thread, geração de mensagem de commit e orquestração de skills. O modelo auxiliar mudou entre versões do cliente, então o opencodex intercepta todos os modelos deste conjunto: {models}. Ative para redirecionar essas chamadas ao modelo escolhido.", "models.shadowCallIntercept": "Shadow Call Intercept", "models.shadowCallInterceptHint": "Intercepta as chamadas auxiliares em segundo plano do Codex App ({models}) para títulos e mensagens de commit e as redireciona ao modelo escolhido.", + "models.autoReviewOverride": "Substituição da revisão automática", + "models.autoReviewOverrideHint": "Define o modelo que o Codex usa na revisão automática (revisor de aprovação). Aplicado a todas as linhas do catálogo na sincronização; um autoReviewModel no provedor continua tendo prioridade nas próprias linhas.", + "models.autoReviewPending": "Pendente", "dash.sidecarBackend": "Backend", "dash.sidecarModel": "Modelo", "dash.backendAuto": "Automático", diff --git a/gui/src/i18n/ru.ts b/gui/src/i18n/ru.ts index daaa5537494..cced4e208c0 100644 --- a/gui/src/i18n/ru.ts +++ b/gui/src/i18n/ru.ts @@ -489,6 +489,9 @@ export const ru: Record = { "dash.shadowCallTooltip": "Codex App в фоновом режиме вызывает служебную модель для генерации заголовков тредов, сообщений коммитов и оркестрации навыков. Эта модель менялась между версиями клиента, поэтому opencodex перехватывает весь набор: {models}. Включите функцию, чтобы перенаправлять такие вызовы на выбранную вами модель.", "models.shadowCallIntercept": "Перехват теневых вызовов", "models.shadowCallInterceptHint": "Перехватывает фоновые служебные вызовы Codex App ({models}: заголовки, сообщения коммитов) и перенаправляет их на выбранную вами модель.", + "models.autoReviewOverride": "Переопределение авто-проверки", + "models.autoReviewOverrideHint": "Задаёт модель, которую Codex использует для автоматической проверки (ревьюера подтверждений). Применяется ко всем строкам каталога при синхронизации; autoReviewModel на уровне провайдера по-прежнему имеет приоритет.", + "models.autoReviewPending": "Ожидание", "dash.sidecarBackend": "Бэкенд", "dash.sidecarModel": "Модель", "dash.backendAuto": "Авто", diff --git a/gui/src/i18n/tr.ts b/gui/src/i18n/tr.ts index 1286bdc85df..02458e1bc3c 100644 --- a/gui/src/i18n/tr.ts +++ b/gui/src/i18n/tr.ts @@ -490,6 +490,9 @@ export const tr: Record = { "dash.shadowCallTooltip": "Codex App arka planda başlık ve commit mesajı oluşturmak için yardımcı çağrılar yapar (orijinal {models}). Etkinleştirerek bunları seçtiğiniz modele yönlendirebilirsiniz.", "models.shadowCallIntercept": "Gölge Çağrı Yakalama", "models.shadowCallInterceptHint": "Codex App'in başlıklar ve commit mesajları için yaptığı arka plan çağrılarını ({models}) yakalar ve seçtiğiniz modele yönlendirir.", + "models.autoReviewOverride": "Otomatik inceleme geçersiz kılma", + "models.autoReviewOverrideHint": "Codex’in otomatik incelemesi (onay gözden geçiren) için kullanılacak modeli seçer. Katalog eşitlemesinde tüm satırlara uygulanır; sağlayıcıya özel autoReviewModel kendi satırlarında yine önceliklidir.", + "models.autoReviewPending": "Beklemede", "dash.sidecarBackend": "Arka uç", "dash.sidecarModel": "Model", "dash.backendAuto": "Otomatik", diff --git a/gui/src/i18n/vi.ts b/gui/src/i18n/vi.ts index 5840c2613bc..3d64a61c4fd 100644 --- a/gui/src/i18n/vi.ts +++ b/gui/src/i18n/vi.ts @@ -488,6 +488,9 @@ export const vi: Record = { "dash.shadowCallTooltip": "Ứng dụng Codex thực hiện các lệnh gọi helper ngầm để tạo tiêu đề chuỗi, tạo commit message và điều phối các kĩ năng. Helper model có thể thay đổi giữa các phiên bản client, do đó opencodex chặn mọi model trong tập hợp này: {models}. Hãy bật tính năng này để chuyển hướng các lệnh gọi đó đến model bạn chọn.", "models.shadowCallIntercept": "Shadow Call Intercept", "models.shadowCallInterceptHint": "Chặn các lệnh gọi helper nền ({models}) của ứng dụng Codex để tạo tiêu đề và commit messages, và chuyển hướng chúng đến model bạn chọn.", + "models.autoReviewOverride": "Ghi đè duyệt tự động", + "models.autoReviewOverrideHint": "Chọn model Codex dùng cho việc duyệt tự động (mô hình đánh giá phê duyệt). Áp dụng cho mọi hàng trong catalog khi đồng bộ; autoReviewModel ở cấp nhà cung cấp vẫn được ưu tiên trên các hàng đó.", + "models.autoReviewPending": "Đang chờ", "dash.sidecarBackend": "Backend", "dash.sidecarModel": "Model", "dash.backendAuto": "Tự động", diff --git a/gui/src/i18n/zh-TW.ts b/gui/src/i18n/zh-TW.ts index 84f4f7d41ae..79264da0090 100644 --- a/gui/src/i18n/zh-TW.ts +++ b/gui/src/i18n/zh-TW.ts @@ -365,6 +365,9 @@ export const zhTW: Record = { "dash.shadowCallTooltip": "Codex 應用會為執行緒標題生成、提交訊息生成與技能編排發出背景 helper 呼叫。helper 模型因客戶端版本而異，因此 opencodex 攔截此集合中的每個模型：{models}。啟用此選項可將這些呼叫重定向到您選擇的模型。", "models.shadowCallIntercept": "影子呼叫攔截", "models.shadowCallInterceptHint": "攔截 Codex 應用的背景 helper 呼叫（{models}）以生成標題與提交訊息，並將它們重定向到您選擇的模型。", + "models.autoReviewOverride": "自動審查覆寫", + "models.autoReviewOverrideHint": "指定 Codex 自動審查（核准審查模型）所使用的模型。同步型錄時套用到所有資料列；提供者層級的 autoReviewModel 在這些資料列上仍具優先權。", + "models.autoReviewPending": "待定", "dash.sidecarBackend": "後端", "dash.sidecarModel": "模型", "dash.backendAuto": "自動", diff --git a/gui/src/i18n/zh.ts b/gui/src/i18n/zh.ts index 184b2722cdf..6250ce639b8 100644 --- a/gui/src/i18n/zh.ts +++ b/gui/src/i18n/zh.ts @@ -484,6 +484,9 @@ export const zh: Record = { "dash.shadowCallTooltip": "Codex 应用会在后台调用辅助模型来生成线程标题、提交消息以及进行技能编排。该模型随客户端版本变化，因此 opencodex 会同时拦截这些模型：{models}。启用此选项可将这些调用重定向到您选择的模型。", "models.shadowCallIntercept": "影子调用拦截", "models.shadowCallInterceptHint": "拦截 Codex 应用的后台辅助调用（{models}）并重定向到所选模型。", + "models.autoReviewOverride": "自动审查覆盖", + "models.autoReviewOverrideHint": "指定 Codex 自动审查（审批审查模型）使用的模型。同步目录时应用于所有行；提供方级的 autoReviewModel 在这些行上仍然优先。", + "models.autoReviewPending": "待定", "dash.sidecarBackend": "后端", "dash.sidecarModel": "模型", "dash.backendAuto": "自动", diff --git a/gui/src/pages/Models.tsx b/gui/src/pages/Models.tsx index 6c0a446fb81..45362ca760b 100644 --- a/gui/src/pages/Models.tsx +++ b/gui/src/pages/Models.tsx @@ -76,6 +76,7 @@ import { freeOnlyInForce, modelPricingKnown, REASONING_EFFORT_LEVELS, + routedModelOptions, type ModelRow, type ProviderContextCapsResponse, type ShadowCallData, @@ -87,8 +88,9 @@ import { SUBAGENT_SURFACE_GUIDE_URL, readSubagentSurfaceAdvisory } from "../suba import { shadowCallModelOptions } from "./dashboard-shared"; import { shadowSourceModelBadge, shadowSourceModelLabel } from "./shadow-call-source"; import { ModelCatalogDelivery } from "./models-catalog-state"; -import { CustomModelsSummary, InfoHint, ModelsSettingsPanel } from "./models-settings-panel"; +import { CustomModelsSummary, InfoHint, ModelsSettingsPanel, SettingsRow } from "./models-settings-panel"; import { modelsSettingsSummary } from "./models-settings-summary"; +import { useAutoReviewSettings } from "./use-auto-review-settings"; type CachedModelsPage = { models: ModelRow[]; @@ -326,6 +328,13 @@ export default function Models({ apiBase, restartEpoch = 0, connected = false, c setStatus(message); setFeedbackGen(g => g + 1); }, []); + const { data: autoReview, saving: autoReviewSaving, load: loadAutoReview, save: saveAutoReview } = + useAutoReviewSettings(apiBase, t, publishFeedback); + useEffect(() => { + if (catalogActive) return; + // A hidden mounted panel may still hold the previous server's reviewer settings. + void loadAutoReview(); + }, [apiBase, catalogActive, loadAutoReview]); // Transient action feedback as a fixed toast: appearing or auto-clearing it never shifts // the workspace below (the old inline Notice pushed the whole model grid down by its // height on every apply). The timer itself just clears the status again. @@ -474,6 +483,11 @@ export default function Models({ apiBase, restartEpoch = 0, connected = false, c ); }, [models, shadowCall?.model, shadowCall?.sourceModels, shadowModelOptions]); + const autoReviewOptions = useMemo( + () => [{ value: "", label: "—" }, ...routedModelOptions(shadowModelOptions, models, autoReview?.model)], + [models, shadowModelOptions, autoReview?.model], + ); + const loadShadowCall = useCallback(async () => { const bounded = createBoundedFetch(15_000); try { @@ -722,6 +736,7 @@ export default function Models({ apiBase, restartEpoch = 0, connected = false, c if (!catalogActive) return; const timeout = window.setTimeout(() => { void loadShadowCall(); + void loadAutoReview(); void loadV2(); // Preset previews belong to the same tab. Loaded once rather than polled: the rules are // shipped code and the catalog poll above already refreshes the rows they describe. @@ -747,7 +762,7 @@ export default function Models({ apiBase, restartEpoch = 0, connected = false, c // gui/.oxlintrc.json (override) and gui/doctor.config.json (ignore.overrides). An in-file // react-doctor-disable comment was tried and removed - it changed nothing, and // react/react-compiler penalises a component for carrying suppressions at all. - }, [catalogActive, loadShadowCall, loadV2]); + }, [catalogActive, loadShadowCall, loadAutoReview, loadV2]); const groups = useMemo( () => buildProviderModelGroups(models, providers), @@ -1949,27 +1964,31 @@ export default function Models({ apiBase, restartEpoch = 0, connected = false, c <>
{modelDiscovery && ( -
- {t("models.newPolicyGlobal")} + void saveModelDiscovery(modelDiscovery.policy === "off" ? "on" : "off")} label={t("models.newPolicyGlobal")} /> -
+ )} -
+ void setDefaultAliases(!aliases.defaults.global)} label={t("models.useDefaultAliasesGlobal")} /> -
-
- {t("models.shadowCallIntercept")} ⓘ + + ⓘ} busy={!shadowCall}> {t("models.shadowCallOriginal", { models: shadowSourceModelBadge(shadowCall?.sourceModels) })} void saveShadowCall({ enabled: !shadowCall?.enabled })} disabled={!shadowCall || shadowCallSaving} label={t("models.shadowCallIntercept")} />
void saveAutoReview({ model: v })} disabled={!autoReview || autoReviewSaving || !autoReview.enabled} label={t("models.autoReviewOverride")} /> +
+ {(v2Loading || v2) && ( -
- {t("models.v2Label")} +
{(["v1", "default", "v2"] as const).map(mode => ( -
+ )} {v2 && v2.multiAgentMode === "v2" && ( -
-
- {t("models.keepNativeOnV1")} - void setKeepNativeChatGptOnV1(!v2.keepNativeChatGptOnV1)} - disabled={v2Busy} - label={t("models.keepNativeOnV1")} - /> - - - - -
-
+ + void setKeepNativeChatGptOnV1(!v2.keepNativeChatGptOnV1)} + disabled={v2Busy} + label={t("models.keepNativeOnV1")} + /> + + + + + )} {pendingSurface && ( {controlsBlock} diff --git a/gui/src/pages/models-settings-panel.tsx b/gui/src/pages/models-settings-panel.tsx index f1bfb366a62..a790c94e12c 100644 --- a/gui/src/pages/models-settings-panel.tsx +++ b/gui/src/pages/models-settings-panel.tsx @@ -82,3 +82,28 @@ export function InfoHint({ text }: { text: string }) { ); } + +/** + * One row of the folded settings panel: label on the left, controls pinned to the right + * edge. Six rows share this shape, so the wrapper lives here rather than being repeated + * in Models.tsx where each copy drifts independently. + */ +export function SettingsRow({ label, hint, busy, muted = true, caption, children }: { + label: ReactNode; + hint?: ReactNode; + busy?: boolean; + muted?: boolean; + caption?: boolean; + children: ReactNode; +}) { + const labelClass = ["models-row-label", muted ? "muted" : "", caption ? "text-caption" : "text-control"].filter(Boolean).join(" "); + return ( +
+ + {label} + {hint ? <> {hint} : null} + +
{children}
+
+ ); +} diff --git a/gui/src/pages/models-settings-summary.ts b/gui/src/pages/models-settings-summary.ts index 1994b6a88ce..997bae4956a 100644 --- a/gui/src/pages/models-settings-summary.ts +++ b/gui/src/pages/models-settings-summary.ts @@ -43,6 +43,9 @@ export interface ModelsSettingsState { keepNativeOnV1?: boolean; shadowEnabled: boolean; shadowModel?: string; + /** Global auto-review override; folded whenever enabled, a blank model reading as pending. */ + autoReviewEnabled?: boolean; + autoReviewModel?: string; windowOn: boolean; windowValue: number; /** The SAVED picker mode — an unapplied draft is not the catalog's state. */ @@ -81,6 +84,9 @@ export function modelsSettingsSummary(t: TFn, state: ModelsSettingsState): Setti label: t("models.shadowCallIntercept"), value: state.shadowEnabled && state.shadowModel ? state.shadowModel : off, }); + if (state.autoReviewEnabled) { + items.push({ id: "auto-review", label: t("models.autoReviewOverride"), value: state.autoReviewModel || t("models.autoReviewPending") }); + } items.push({ id: "window", label: t("models.contextCapLabel"), value: state.windowOn ? fmtK(state.windowValue) : off }); const pickerKey = PICKER_MODE_KEYS[state.pickerMode]; items.push({ id: "order", label: t("models.pickerOrder.label"), value: pickerKey ? t(pickerKey) : state.pickerMode }); diff --git a/gui/src/pages/models-shared.ts b/gui/src/pages/models-shared.ts index 8dc8c4c73aa..24a5037be45 100644 --- a/gui/src/pages/models-shared.ts +++ b/gui/src/pages/models-shared.ts @@ -168,6 +168,12 @@ export interface ShadowCallData { sourceModels?: string[]; } +/** The global auto-review (approval reviewer) override the Models settings panel writes. */ +export interface AutoReviewData { + enabled: boolean; + model: string; +} + export const CAP_OPTIONS = Array.from({ length: 18 }, (_, i) => 100_000 + i * 50_000); // 100k … 950k export const CAP_OPTION_SET = new Set(CAP_OPTIONS); /** @@ -230,6 +236,27 @@ export function activeModelOptions( return options; } +/** + * Narrow the catalog options to the routed rows an auto-review override may name. + * + * The reviewer selector has to resolve to a provider-namespaced catalog row; native rows are + * dropped because Codex's own default already covers them and choosing one would pin the + * subscription model the override exists to avoid. A value saved before its row left the list + * stays selectable, or the control would read back blank and the next save would erase it. + */ +export function routedModelOptions( + visible: readonly { value: string; label: string }[], + models: readonly ModelRow[], + saved?: string, +): { value: string; label: string }[] { + const native = new Set(models.filter(model => model.native === true).map(model => model.namespaced)); + const options = visible.filter(option => !native.has(option.value)); + if (saved && !options.some(option => option.value === saved)) { + options.push({ value: saved, label: visible.find(option => option.value === saved)?.label ?? saved }); + } + return options; +} + /** `null` = no preference yet → caller should default to all groups collapsed. */ export function readCollapsedProviders(storage: StorageLike = localStorage): Set | null { try { diff --git a/gui/src/pages/use-auto-review-settings.ts b/gui/src/pages/use-auto-review-settings.ts new file mode 100644 index 00000000000..75215274080 --- /dev/null +++ b/gui/src/pages/use-auto-review-settings.ts @@ -0,0 +1,110 @@ +import { useCallback, useLayoutEffect, useRef, useState } from "react"; +import { createBoundedFetch, type BoundedFetch } from "../bounded-fetch"; +import { readJsonIfOk, readJsonOrThrow } from "../fetch-json"; +import type { TFn } from "../i18n/shared"; +import type { AutoReviewData } from "./models-shared"; + +export interface AutoReviewSettingsController { + data: AutoReviewData | null; + saving: boolean; + load: (options?: { reconcile?: boolean }) => Promise; + save: (patch: Partial) => Promise; +} + +export function useAutoReviewSettings( + apiBase: string, + t: TFn, + publishFeedback: (ok: boolean, message: string) => void, +): AutoReviewSettingsController { + const [state, setState] = useState<{ apiBase: string; data: AutoReviewData | null; saving: boolean }>({ + apiBase, data: null, saving: false, + }); + const data = state.apiBase === apiBase ? state.data : null; + const saving = state.apiBase === apiBase && state.saving; + const requests = useRef<{ read: BoundedFetch | null; write: BoundedFetch | null; generation: number; apiBase: string }>({ + read: null, write: null, generation: 0, apiBase, + }); + + useLayoutEffect(() => { + const scope = requests.current; + scope.apiBase = apiBase; + scope.generation++; + return () => { + scope.generation++; + scope.read?.controller.abort(); + scope.read?.clear(); + scope.read = null; + scope.write?.controller.abort(); + scope.write?.clear(); + scope.write = null; + }; + }, [apiBase]); + + const load = useCallback(async ({ reconcile = false }: { reconcile?: boolean } = {}) => { + const scope = requests.current; + if (scope.write && !reconcile) return null; + scope.read?.controller.abort(); + scope.read?.clear(); + const bounded = createBoundedFetch(15_000); + const generation = scope.generation; + scope.read = bounded; + try { + const response = await fetch(`${apiBase}/api/auto-review-settings`, { signal: bounded.signal }); + const next = await readJsonIfOk(response); + if (!next || typeof next.enabled !== "boolean" || typeof next.model !== "string") return null; + if (!bounded.signal.aborted && scope.apiBase === apiBase && scope.generation === generation && scope.read === bounded + && (!scope.write || reconcile)) { + setState({ apiBase, data: next, saving: scope.write !== null }); + return next; + } + return null; + } catch { /* old server / network: keep the row disabled */ return null; } + finally { + bounded.clear(); + if (scope.read === bounded) scope.read = null; + } + }, [apiBase]); + + const save = useCallback(async (patch: Partial) => { + const scope = requests.current; + if (!data || saving || scope.apiBase !== apiBase || scope.write) return; + const confirmed = data; + const bounded = createBoundedFetch(15_000); + scope.write = bounded; + const generation = ++scope.generation; + scope.read?.controller.abort(); + scope.read?.clear(); + scope.read = null; + setState({ apiBase, data: { ...confirmed, ...patch }, saving: true }); + try { + const response = await fetch(`${apiBase}/api/auto-review-settings`, { + method: "PUT", headers: { "Content-Type": "application/json" }, signal: bounded.signal, + body: JSON.stringify(patch), + }); + const next = await readJsonOrThrow(response, t("models.saveFailed")); + if (!next || next.ok !== true || typeof next.enabled !== "boolean" || typeof next.model !== "string") { + throw new Error(t("models.saveFailed")); + } + if (scope.apiBase !== apiBase || scope.generation !== generation) return; + setState({ apiBase, data: { enabled: next.enabled, model: next.model }, saving: true }); + if (!next.catalogRefresh || next.catalogRefresh.status !== "committed") { + publishFeedback(false, t("codexAuth.catalogRefreshPending")); + } + } catch (error) { + if (scope.apiBase === apiBase && scope.generation === generation) { + const refreshed = await load({ reconcile: true }); + if (scope.apiBase !== apiBase || scope.generation !== generation) return; + setState({ apiBase, data: refreshed ?? confirmed, saving: true }); + publishFeedback(false, error instanceof Error && error.message ? error.message : t("models.saveFailed")); + } + } finally { + bounded.clear(); + if (scope.write === bounded) scope.write = null; + if (scope.apiBase === apiBase && scope.generation === generation) { + setState(current => current.apiBase === apiBase ? { ...current, saving: false } : current); + } + } + }, [apiBase, data, saving, t, publishFeedback, load]); + + return { data, saving, load, save }; +} diff --git a/gui/src/styles-models-workspace.css b/gui/src/styles-models-workspace.css index faa06b6f8d6..d1272e36d2a 100644 --- a/gui/src/styles-models-workspace.css +++ b/gui/src/styles-models-workspace.css @@ -161,28 +161,47 @@ min-width: 0; } -/* Controls row: shadow call on the left, sub-agent mode pinned right. - Right group does not move when the model name length changes. */ +/* + The settings panel is one stack of uniform rows: each row is a label on the + left and its control cluster on the right, so every switch and picker lines up + on the same right edge no matter how long a model name or hint is. Rows span + the full panel width rather than pairing into columns, which kept the two + model pickers at ragged widths and left the sub-agent chips floating. +*/ .models-control-top-row { display: grid; - grid-template-columns: minmax(0, 1fr) auto; - align-items: center; - column-gap: clamp(var(--space-3), 2vw, var(--space-6)); - row-gap: var(--space-2); + grid-template-columns: minmax(0, 1fr); + row-gap: var(--space-1); margin-bottom: var(--space-3); min-width: 0; } -.models-shadow-row { +/* One labelled control: label group left, action group right, both vertically centered. */ +.models-control-row { display: flex; align-items: center; + justify-content: space-between; + gap: var(--space-3); + flex-wrap: wrap; + min-width: 0; + min-height: 32px; +} + +.models-row-label { + display: inline-flex; + align-items: center; gap: var(--space-2); min-width: 0; - flex-wrap: nowrap; } -.models-shadow-label { - white-space: nowrap; +.models-row-actions { + display: inline-flex; + align-items: center; + justify-content: flex-end; + gap: var(--space-2); + flex-wrap: wrap; + flex: 0 1 auto; + min-width: 0; } .models-shadow-warning { @@ -215,18 +234,6 @@ white-space: nowrap; } -.models-v2-mode-row { - display: flex; - align-items: center; - gap: var(--space-2); - flex: 0 0 auto; - min-width: 0; - flex-wrap: nowrap; - justify-self: end; - /* Keep the Sub-agent chip row on the same cap-height as Shadow Call controls. */ - min-height: 28px; -} - .models-v2-detail-row { gap: var(--space-2); margin-bottom: var(--space-2); @@ -234,37 +241,6 @@ align-items: center; } -/* - "Keep ChatGPT on v1" is a sub-setting of the Sub-agent surface control, not a - peer of it. It sits in the right-hand grid column directly under the v1/base/v2 - chips and stays right-aligned with them, so it reads as a qualifier on the - selected mode instead of floating in the empty space beside Shadow Call. -*/ -.models-v2-keep-native-row { - grid-column: 2; - justify-self: end; - display: flex; - justify-content: flex-end; - min-width: 0; - /* Pull up against the mode row's own min-height so the pair reads as one block. */ - margin-top: calc(var(--space-1) * -1); -} - -.models-v2-keep-native { - display: inline-flex; - align-items: center; - gap: var(--space-2); - min-width: 0; -} - -.models-v2-keep-native-label { - color: var(--muted); - white-space: nowrap; - min-width: 0; - overflow: hidden; - text-overflow: ellipsis; -} - /* Sits in the same 24px gutter as the Sub-agent help button directly above it, so both rows end on one right edge. */ .models-v2-keep-native-info { @@ -572,37 +548,12 @@ /* ── Responsive ───────────────────────────────────────── */ @media (max-width: 1160px) { - .models-control-top-row { - grid-template-columns: 1fr; - } - - .models-shadow-row { - flex: 1 1 100%; - flex-wrap: wrap; - } - .models-shadow-model-slot { flex: 1 1 12rem; width: auto; min-width: min(100%, 10rem); max-width: 100%; } - - .models-v2-mode-row { - flex-wrap: wrap; - justify-self: start; - } - - .models-v2-keep-native-row { - grid-column: 1; - justify-self: start; - justify-content: flex-start; - margin-top: 0; - } - - .models-v2-keep-native { - white-space: normal; - } } /* @@ -762,6 +713,8 @@ } .models-settings-body > .models-control-top-row { margin-bottom: var(--space-3); } .models-settings-body .models-cap-row { margin-bottom: var(--space-3); } +/* The two cap rows put their controls on the same right edge as the rows above. */ +.models-settings-body .models-cap-row > .muted:first-child { margin-right: auto; } .models-settings-body > .models-aliases-card { margin: var(--space-2) 0 0; } .models-info-hint { @@ -776,22 +729,3 @@ .models-collapse-controls { align-items: center; flex-wrap: wrap; } .models-collapse-controls > .models-custom-summary { margin: 0 0 0 auto; } -/* - Wide workspace: the open panel lays its rows out in two columns instead of one tall - stack. Rows that carry their own internal grid or a list span the full width. -*/ -@container models-workspace (min-width: 1000px) { - .models-settings-body { - display: grid; - grid-template-columns: repeat(2, minmax(0, 1fr)); - column-gap: var(--space-6); - row-gap: 0; - align-items: start; - } - .models-settings-body > .models-control-top-row, - .models-settings-body > .models-v2-detail-row, - .models-settings-body > .picker-order-editor, - .models-settings-body > .models-aliases-card { - grid-column: 1 / -1; - } -} diff --git a/gui/tests/models-auto-review-override.test.tsx b/gui/tests/models-auto-review-override.test.tsx new file mode 100644 index 00000000000..424c52f676c --- /dev/null +++ b/gui/tests/models-auto-review-override.test.tsx @@ -0,0 +1,65 @@ +import { expect, test } from "bun:test"; +import { modelsSettingsSummary } from "../src/pages/models-settings-summary"; +import { routedModelOptions, type ModelRow } from "../src/pages/models-shared"; +import type { TFn } from "../src/i18n/shared"; + +/** + * The Models settings panel folds the global auto-review override. Two properties matter: + * the dropdown offers routed rows (the selector must name a provider-namespaced catalog row), + * and the summary keeps an enabled control visible, including its pending model state. + */ +const t = ((key: string) => key) as unknown as TFn; + +const base = { + multiAgentMode: "v1" as const, + shadowEnabled: true, + shadowModel: "xai/grok-4.5", + windowOn: false, + windowValue: 350_000, + pickerMode: "most-used", + newModelsOff: false, + aliasesOn: false, +}; + +function row(partial: Pick & Partial): ModelRow { + return { disabled: false, ...partial }; +} + +test("summary names the auto-review override only while it is on", () => { + expect(modelsSettingsSummary(t, base).some(item => item.id === "auto-review")).toBe(false); + const on = modelsSettingsSummary(t, { + ...base, + autoReviewEnabled: true, + autoReviewModel: "9router/ocg-muse-spark-1.3-contributor", + }); + expect(on.find(item => item.id === "auto-review")).toEqual({ + id: "auto-review", + label: "models.autoReviewOverride", + value: "9router/ocg-muse-spark-1.3-contributor", + }); +}); + +test("an enabled override with no model reads as pending instead of vanishing", () => { + const pending = modelsSettingsSummary(t, { ...base, autoReviewEnabled: true, autoReviewModel: "" }); + expect(pending.find(item => item.id === "auto-review")).toEqual({ + id: "auto-review", + label: "models.autoReviewOverride", + value: "models.autoReviewPending", + }); +}); + +test("the override dropdown offers routed rows and keeps a saved native value visible", () => { + const models = [ + row({ provider: "openai", id: "gpt-5.6-sol", namespaced: "gpt-5.6-sol", native: true }), + row({ provider: "9router", id: "ocg-muse-spark-1.3-contributor", namespaced: "9router/ocg-muse-spark-1.3-contributor" }), + ]; + const visible = [ + { value: "gpt-5.6-sol", label: "GPT-5.6-Sol" }, + { value: "9router/ocg-muse-spark-1.3-contributor", label: "muse-spark" }, + ]; + expect(routedModelOptions(visible, models).map(option => option.value)) + .toEqual(["9router/ocg-muse-spark-1.3-contributor"]); + // A value saved by hand in config.json must stay selectable, or the row would read as empty. + expect(routedModelOptions(visible, models, "gpt-5.6-sol").map(option => option.value).sort()) + .toEqual(["9router/ocg-muse-spark-1.3-contributor", "gpt-5.6-sol"]); +}); diff --git a/gui/tests/models-auto-review-settings.test.tsx b/gui/tests/models-auto-review-settings.test.tsx new file mode 100644 index 00000000000..1b1b026d8df --- /dev/null +++ b/gui/tests/models-auto-review-settings.test.tsx @@ -0,0 +1,261 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { Window } from "happy-dom"; +import { en } from "../src/i18n/en"; +import { interpolate, type TFn } from "../src/i18n/shared"; +import { useAutoReviewSettings, type AutoReviewSettingsController } from "../src/pages/use-auto-review-settings"; + +const t: TFn = (key, vars) => interpolate(en[key], vars); +const globals = ["document", "window", "navigator", "fetch", "IS_REACT_ACT_ENVIRONMENT"] as const; +let previous: Record; +let win: Window; +let host: HTMLElement; +let root: Root; +let controller: AutoReviewSettingsController; +let requests: Array<{ url: string; init?: RequestInit }>; +let feedback: Array<[boolean, string]>; +let respond: (url: string, init?: RequestInit) => Promise; + +function response(payload: unknown, status = 200): Response { return Response.json(payload, { status }); } +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise(r => { resolve = r; }); + return { promise, resolve }; +} +function Harness({ apiBase }: { apiBase: string }) { + controller = useAutoReviewSettings(apiBase, t, (ok, message) => feedback.push([ok, message])); + return null; +} +async function paint(apiBase: string) { + await act(async () => { root.render(); }); +} + +beforeEach(() => { + previous = Object.fromEntries(globals.map(key => [key, Reflect.get(globalThis, key)])); + win = new Window({ url: "http://localhost/" }); + Object.defineProperties(globalThis, { + document: { configurable: true, value: win.document }, + window: { configurable: true, value: win }, + navigator: { configurable: true, value: win.navigator }, + IS_REACT_ACT_ENVIRONMENT: { configurable: true, value: true }, + }); + requests = []; feedback = []; + respond = async () => response({ enabled: false, model: "" }); + Object.defineProperty(globalThis, "fetch", { configurable: true, value: async (input: string, init?: RequestInit) => { + requests.push({ url: String(input), init }); + return respond(String(input), init); + } }); + host = win.document.createElement("div") as unknown as HTMLElement; + win.document.body.appendChild(host as never); + root = createRoot(host); +}); + +afterEach(async () => { + await act(async () => { root.unmount(); }); + for (const key of globals) Object.defineProperty(globalThis, key, { configurable: true, value: previous[key] }); + await win.happyDOM.close(); +}); + +test("A-to-B-to-A ignores the first base's late GET result", async () => { + const abandoned = deferred(); + const currentA = deferred(); + let aReads = 0; + respond = async url => { + if (url.startsWith("/a/")) return ++aReads === 1 ? abandoned.promise : currentA.promise; + return response({ enabled: false, model: "server-b/model" }); + }; + + await paint("/a"); + let abandonedRead!: Promise; + await act(async () => { abandonedRead = controller.load(); }); + const oldSignal = requests[0]!.init!.signal!; + await paint("/b"); + expect(oldSignal.aborted).toBe(true); + await act(async () => { await controller.load(); }); + expect(controller.data).toEqual({ enabled: false, model: "server-b/model" }); + + await paint("/a"); + let freshRead!: Promise; + await act(async () => { freshRead = controller.load(); }); + expect(controller.data).toBeNull(); + await act(async () => { abandoned.resolve(response({ enabled: true, model: "stale-a/model" })); await abandonedRead; }); + expect(controller.data).toBeNull(); + await act(async () => { currentA.resolve(response({ enabled: false, model: "fresh-a/model" })); await freshRead; }); + expect(controller.data).toEqual({ enabled: false, model: "fresh-a/model" }); +}); + +test("a GET overlapping a save cannot replace its state or re-enable writes", async () => { + const staleRead = deferred(); + const write = deferred(); + let reads = 0; + respond = async (_url, init) => { + if (init?.method === "PUT") return write.promise; + return ++reads === 1 + ? response({ enabled: false, model: "confirmed/model" }) + : staleRead.promise; + }; + await paint("/a"); + await act(async () => { await controller.load(); }); + + let pendingRead!: ReturnType; + await act(async () => { pendingRead = controller.load(); }); + let pendingSave!: Promise; + await act(async () => { pendingSave = controller.save({ enabled: true }); }); + await act(async () => { staleRead.resolve(response({ enabled: false, model: "stale/model" })); await pendingRead; }); + const dataAfterRead = controller.data; + const savingAfterRead = controller.saving; + let secondSave!: Promise; + await act(async () => { secondSave = controller.save({ model: "second/model" }); }); + const writes = requests.filter(request => request.init?.method === "PUT").length; + await act(async () => { + write.resolve(response({ ok: true, enabled: true, model: "confirmed/model", catalogRefresh: { status: "committed" } })); + await Promise.all([pendingSave, secondSave]); + }); + + expect(dataAfterRead).toEqual({ enabled: true, model: "confirmed/model" }); + expect(savingAfterRead).toBe(true); + expect(writes).toBe(1); +}); + +test("an ordinary GET started during a save is ignored", async () => { + const write = deferred(); + let reads = 0; + respond = async (_url, init) => init?.method === "PUT" + ? write.promise + : response({ enabled: false, model: ++reads === 1 ? "confirmed/model" : "stale/model" }); + await paint("/a"); + await act(async () => { await controller.load(); }); + let pendingSave!: Promise; + await act(async () => { pendingSave = controller.save({ enabled: true }); }); + let readResult: unknown; + await act(async () => { readResult = await controller.load(); }); + const requestCount = requests.length; + const dataAfterRead = controller.data; + const savingAfterRead = controller.saving; + await act(async () => { + write.resolve(response({ ok: true, enabled: true, model: "confirmed/model", catalogRefresh: { status: "committed" } })); + await pendingSave; + }); + + expect(readResult).toBeNull(); + expect(requestCount).toBe(2); + expect(dataAfterRead).toEqual({ enabled: true, model: "confirmed/model" }); + expect(savingAfterRead).toBe(true); +}); + +test("a GET begun before a PUT stays stale if it completes after the PUT", async () => { + const lateRead = deferred(); + let reads = 0; + respond = async (_url, init) => { + if (init?.method === "PUT") return response({ ok: true, enabled: true, model: "saved/model", catalogRefresh: { status: "committed" } }); + return ++reads === 1 ? response({ enabled: false, model: "confirmed/model" }) : lateRead.promise; + }; + await paint("/a"); + await act(async () => { await controller.load(); }); + let pendingRead!: ReturnType; + await act(async () => { pendingRead = controller.load(); }); + const readSignal = requests.at(-1)!.init!.signal!; + await act(async () => { await controller.save({ enabled: true, model: "saved/model" }); }); + expect(readSignal.aborted).toBe(true); + await act(async () => { lateRead.resolve(response({ enabled: false, model: "stale/model" })); await pendingRead; }); + + expect(controller.data).toEqual({ enabled: true, model: "saved/model" }); + expect(controller.saving).toBe(false); +}); + +test("explicit reconciliation refreshes settings without ending a save", async () => { + const write = deferred(); + const reconciliation = deferred(); + let reads = 0; + respond = async (_url, init) => { + if (init?.method === "PUT") return write.promise; + return ++reads === 1 + ? response({ enabled: false, model: "confirmed/model" }) + : reconciliation.promise; + }; + await paint("/a"); + await act(async () => { await controller.load(); }); + let pendingSave!: Promise; + await act(async () => { pendingSave = controller.save({ enabled: true }); }); + let refresh!: ReturnType; + await act(async () => { refresh = controller.load({ reconcile: true }); }); + await act(async () => { + reconciliation.resolve(response({ enabled: false, model: "fresh/model" })); + await refresh; + }); + const dataAfterRefresh = controller.data; + const savingAfterRefresh = controller.saving; + await act(async () => { + write.resolve(response({ ok: true, enabled: true, model: "confirmed/model", catalogRefresh: { status: "committed" } })); + await pendingSave; + }); + + expect(dataAfterRefresh).toEqual({ enabled: false, model: "fresh/model" }); + expect(savingAfterRefresh).toBe(true); +}); + +test("failed save reloads the confirmed server value and reports the error", async () => { + let reads = 0; + respond = async (_url, init) => { + if (init?.method === "PUT") return response({ error: "save rejected" }, 500); + reads++; + return response({ enabled: false, model: reads === 1 ? "initial/model" : "current-on-server/model" }); + }; + await paint("/a"); + await act(async () => { await controller.load(); }); + await act(async () => { await controller.save({ enabled: true }); }); + expect(controller.data).toEqual({ enabled: false, model: "current-on-server/model" }); + expect(feedback).toEqual([[false, "save rejected"]]); +}); + +test("failed save keeps the confirmed value when reconciliation also fails", async () => { + let reads = 0; + respond = async (_url, init) => { + if (init?.method === "PUT") return response({ error: "save rejected" }, 500); + reads++; + return reads === 1 + ? response({ enabled: false, model: "confirmed/model" }) + : response({ error: "offline" }, 503); + }; + await paint("/a"); + await act(async () => { await controller.load(); }); + await act(async () => { await controller.save({ enabled: true }); }); + expect(controller.data).toEqual({ enabled: false, model: "confirmed/model" }); + expect(feedback).toEqual([[false, "save rejected"]]); +}); + +test("saved settings report an uncommitted catalog refresh", async () => { + respond = async (_url, init) => init?.method === "PUT" + ? response({ ok: true, enabled: true, model: "router/reviewer", catalogRefresh: { status: "skipped" } }) + : response({ enabled: false, model: "" }); + await paint("/a"); + await act(async () => { await controller.load(); }); + await act(async () => { await controller.save({ enabled: true, model: "router/reviewer" }); }); + expect(controller.data).toEqual({ enabled: true, model: "router/reviewer" }); + expect(feedback).toEqual([[false, en["codexAuth.catalogRefreshPending"]]]); +}); + +test("a stale save response after a proxy change cannot overwrite the new proxy", async () => { + const write = deferred(); + respond = async (url, init) => { + if (url.startsWith("/a/") && init?.method === "PUT") return write.promise; + if (url.startsWith("/b/")) return response({ enabled: false, model: "server-b/model" }); + return response({ enabled: false, model: "server-a/model" }); + }; + await paint("/a"); + await act(async () => { await controller.load(); }); + let pendingSave!: Promise; + await act(async () => { pendingSave = controller.save({ enabled: true, model: "server-a/reviewer" }); }); + const signal = requests.find(request => request.init?.method === "PUT")!.init!.signal!; + + await paint("/b"); + expect(signal.aborted).toBe(true); + await act(async () => { await controller.load(); }); + await act(async () => { + write.resolve(response({ ok: true, enabled: true, model: "server-a/reviewer", catalogRefresh: { status: "committed" } })); + await pendingSave; + }); + expect(controller.data).toEqual({ enabled: false, model: "server-b/model" }); + expect(feedback).toHaveLength(0); +}); diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index e90f18ff399..b8e8cab1182 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -20,11 +20,11 @@ "cli-anthropic2-account.test.ts": "cli", "anthropic2-catalog.test.ts": "codex-integration", "provider-anthropic-instance-quota.test.ts": "providers", "anthropic2-management.test.ts": "server", "management-sidecar-anthropic-instance.test.ts": "server", "anthropic2-usage-attribution.test.ts": "usage", "vision-anthropic-instance-sidecar.test.ts": "vision", "web-search-anthropic-instance.test.ts": "web-search", "provider-anthropic-instance.test.ts": "providers", "config-anthropic-instance-pool.test.ts": "config", "oauth-anthropic-instance-registration.test.ts": "oauth", "oauth-anthropic-instance-refresh.test.ts": "oauth", - "claude-picker-upload.test.ts": "claude-integration", "claude-picker-linger.test.ts": "claude-integration", "messages-native-scope.test.ts": "claude-integration", "catalog-auto-review-row.test.ts": "codex-integration", + "claude-picker-upload.test.ts": "claude-integration", "claude-picker-linger.test.ts": "claude-integration", "messages-native-scope.test.ts": "claude-integration", "catalog-auto-review-row.test.ts": "codex-integration", "catalog-auto-review-override.test.ts": "codex-integration", "cli-log-view-filter.test.ts": "cli", "cli-usage-model-search.test.ts": "cli", "cli-companion-usage.test.ts": "cli", "cli-account-key-quota.test.ts": "cli", "cli-observe-snapshot.test.ts": "cli", "cli-access-data-plane.test.ts": "cli", "cli-access-key-guard.test.ts": "cli", "cli-access-rename.test.ts": "cli", "cli-access-audio.test.ts": "cli", "cli-access-audio-live.test.ts": "cli", "cli-log-follow.test.ts": "cli", "cli-injection-follow.test.ts": "cli", - "cli-usage-scope.test.ts": "cli", "cli-system-health.test.ts": "cli", "cli-companion-timeline.test.ts": "cli", "server-cli-key-probe-admission.test.ts": "server", + "cli-usage-scope.test.ts": "cli", "cli-system-health.test.ts": "cli", "cli-companion-timeline.test.ts": "cli", "server-cli-key-probe-admission.test.ts": "server", "management-auto-review-settings.test.ts": "server", "management-auto-review-auth.test.ts": "server", "cli-integration-preview.test.ts": "cli", "cli-integration-droid.test.ts": "cli", "cli-integration-journal.test.ts": "cli", "cli-aside-sync.test.ts": "cli", "cli-claude-desktop-profile.test.ts": "cli", "cli-integration-cursor.test.ts": "cli", "cli-remote-workspace-hub.test.ts": "cli", "cli-storage-policy-fields.test.ts": "cli", "cli-link-force.test.ts": "cli", "cli-account-policy.test.ts": "cli", "cli-account-login-options.test.ts": "cli", "cli-logout-runtime.test.ts": "cli", "cli-agent-settings.test.ts": "cli", diff --git a/skills/ocx/references/01_management_surface.md b/skills/ocx/references/01_management_surface.md index 1421cb57a3d..436049ee3f0 100644 --- a/skills/ocx/references/01_management_surface.md +++ b/skills/ocx/references/01_management_surface.md @@ -26,7 +26,7 @@ These answer in the CLI head and never reach the proxy, so they work with nothin | Chapter | Declared capabilities | |---|---| | [lifecycle](01_surface_lifecycle.md) | 12 | -| [providers-models](01_surface_providers-models.md) | 47 | +| [providers-models](01_surface_providers-models.md) | 49 | | [accounts](01_surface_accounts.md) | 40 | | [agents-routing](01_surface_agents-routing.md) | 50 | | [integrations](01_surface_integrations.md) | 41 | @@ -475,6 +475,14 @@ Original invocation order. These headings preserve links to the previous single- [State-changing task](01_surface_providers-models.md#ocx-models-shadow-set) +### `ocx models auto-review status` + +[Read-oriented task](01_surface_providers-models.md#ocx-models-auto-review-status) + +### `ocx models auto-review set` + +[State-changing task](01_surface_providers-models.md#ocx-models-auto-review-set) + ### `ocx alias list` [Read-oriented task](01_surface_providers-models.md#ocx-alias-list) @@ -1389,6 +1397,6 @@ Original invocation order. These headings preserve links to the previous single- ## Counts -- declared capabilities: 335 -- of those, state-changing: 204 +- declared capabilities: 337 +- of those, state-changing: 205 - head-resolved invocations: 2 diff --git a/skills/ocx/references/01_surface_providers-models.md b/skills/ocx/references/01_surface_providers-models.md index 05965bca20b..9d5eb7c9d44 100644 --- a/skills/ocx/references/01_surface_providers-models.md +++ b/skills/ocx/references/01_surface_providers-models.md @@ -8,7 +8,7 @@ Use these declarations to choose a task, then check its flags and authority before execution. Non-mutating probes may still contact providers, consume quota or refresh caches. -Declared capabilities: 47. +Declared capabilities: 49. ### `ocx models price` @@ -800,6 +800,45 @@ JSON mode: `payload`. - At least a model or --enabled is required; - clears the model with an empty string. Enabling shadow calls can cause subsequent inference calls. +### `ocx models auto-review status` + +Usage: `ocx models auto-review status [--json]` + +Read the global auto-review model and enabled state. + +State-changing: no. + +| Method | Route | +|---|---| +| GET | `/api/auto-review-settings` | + +| Flag | Value | Meaning | +|---|---|---| +| `--json` | boolean | Emit auto-review settings. | + +JSON mode: `payload`. + +### `ocx models auto-review set` + +Usage: `ocx models auto-review set [model|-] [--enabled ] [--json]` + +Set the global auto-review model or enabled state. + +State-changing: yes. + +| Method | Route | +|---|---| +| PUT | `/api/auto-review-settings` | + +| Flag | Value | Meaning | +|---|---|---| +| `--enabled` | string | on or off; takes a value. | +| `--json` | boolean | Emit the settings receipt. | + +JSON mode: `payload`. + +- At least a model or --enabled is required; - clears the model with an empty string. Human output points to ocx sync when a catalog refresh can be retried and asks operators to resolve a failed refresh before syncing. + ### `ocx alias list` Usage: `ocx alias list [--json]` diff --git a/src/cli/capabilities-provider-models.ts b/src/cli/capabilities-provider-models.ts index 00749e71c03..abf1b61cfc9 100644 --- a/src/cli/capabilities-provider-models.ts +++ b/src/cli/capabilities-provider-models.ts @@ -207,6 +207,17 @@ export const PROVIDER_MODEL_CAPABILITIES: readonly Capability[] = [ flags: [{ name: "--enabled", value: "string", summary: "on or off; takes a value." }, { name: "--json", value: "boolean", summary: "Emit the settings receipt." }], details: ["At least a model or --enabled is required; - clears the model with an empty string. Enabling shadow calls can cause subsequent inference calls."], }, + { + command: ["models", "auto-review", "status"], summary: "Read the global auto-review model and enabled state.", + usage: "ocx models auto-review status [--json]", routes: [{ method: "GET", path: "/api/auto-review-settings" }], mutates: false, json: "payload", + flags: [{ name: "--json", value: "boolean", summary: "Emit auto-review settings." }], + }, + { + command: ["models", "auto-review", "set"], summary: "Set the global auto-review model or enabled state.", + usage: "ocx models auto-review set [model|-] [--enabled ] [--json]", routes: [{ method: "PUT", path: "/api/auto-review-settings" }], mutates: true, json: "payload", + flags: [{ name: "--enabled", value: "string", summary: "on or off; takes a value." }, { name: "--json", value: "boolean", summary: "Emit the settings receipt." }], + details: ["At least a model or --enabled is required; - clears the model with an empty string. Human output points to ocx sync when a catalog refresh can be retried and asks operators to resolve a failed refresh before syncing."], + }, { command: ["alias", "list"], summary: "Read provider and model aliases.", usage: "ocx alias list [--json]", routes: [{ method: "GET", path: "/api/aliases" }], mutates: false, json: "payload", diff --git a/src/cli/models-runtime-subcommands.ts b/src/cli/models-runtime-subcommands.ts index edb7de419aa..0a2b6c2f8e4 100644 --- a/src/cli/models-runtime-subcommands.ts +++ b/src/cli/models-runtime-subcommands.ts @@ -28,6 +28,7 @@ export const MODELS_RUNTIME_SUBCOMMANDS = [ "new-arrivals", "context", "shadow", + "auto-review", "order", "display-name", ] as const; diff --git a/src/cli/models-runtime.ts b/src/cli/models-runtime.ts index 86b9cdd25aa..08f29152ccf 100644 --- a/src/cli/models-runtime.ts +++ b/src/cli/models-runtime.ts @@ -48,6 +48,7 @@ const USAGE = `Usage: ocx models new-arrivals [--json] ${MODELS_CONTEXT_USAGE} ocx models shadow [model|-] [--enabled ] [--json] + ocx models auto-review [model|-] [--enabled ] [--json] Prices are USD per 1M tokens. Omitted cache rates default to 0. Price selectors use the exact upstream model ID after the first slash.`; @@ -566,6 +567,39 @@ async function shadow(argv: string[], deps: RuntimeApiDeps): Promise { printData(result, wantsJson, ["Shadow-call settings updated."]); } +async function autoReview(argv: string[], deps: RuntimeApiDeps): Promise { + const args = [...argv]; + const action = (args.shift() ?? "status").toLowerCase(); + const wantsJson = takeFlag(args, "--json"); + if (action === "status") { + rejectArgs(args, USAGE); + const result = await runtimeRequest("/api/auto-review-settings", {}, deps); + printData(result, wantsJson); + return; + } + if (action !== "set") throw new CliUsageError("unknown auto-review action " + action, USAGE); + const modelRaw = args[0] && !args[0].startsWith("--") ? args.shift() : undefined; + const enabled = takeBooleanOption(args, "--enabled"); + rejectArgs(args, USAGE); + const body: Record = {}; + if (modelRaw !== undefined) body.model = modelRaw === "-" ? "" : modelRaw; + if (enabled !== undefined) body.enabled = enabled; + if (Object.keys(body).length === 0) throw new CliUsageError("model and/or --enabled is required", USAGE); + const result = await runtimeRequest<{ catalogRefresh?: { status?: string; retryable?: boolean } }>( + "/api/auto-review-settings", { method: "PUT", body: JSON.stringify(body) }, deps, + ); + const refresh = result.catalogRefresh; + printData(result, wantsJson, [ + "Auto-review settings updated.", + ...(refresh?.status === "failed" + ? ["Settings saved, but the Codex model catalog refresh failed. Resolve the refresh issue before running ocx sync."] + : refresh?.status === "skipped" && refresh.retryable === true + ? ["Settings saved, but the Codex model catalog refresh is pending. Run ocx sync to retry."] + : []), + ]); + if (refresh?.status === "failed") return 1; +} + export async function handleModelsRuntimeCommand(sub: string, argv: string[], deps: RuntimeApiDeps = {}): Promise { if (sub === "order" || sub === "display-name") { const { handleModelsOrderCommand, handleModelsDisplayNameCommand } = await import("./models-order"); @@ -574,7 +608,7 @@ export async function handleModelsRuntimeCommand(sub: string, argv: string[], de // The dispatch below and MODELS_RUNTIME_SUBCOMMANDS must name the same set; // tests/cli/cli-models-runtime-dispatch.test.ts fails if they drift (#3094). if (!isModelsRuntimeSubcommand(sub)) return null; - let action: (() => Promise) | undefined; + let action: (() => Promise) | undefined; if (sub === "live") action = () => live(argv, deps); else if (sub === "price") action = () => price(false, argv, deps); else if (sub === "set-price") action = () => price(true, argv, deps); @@ -589,8 +623,11 @@ export async function handleModelsRuntimeCommand(sub: string, argv: string[], de else if (sub === "new-arrivals") action = () => newArrivals(argv, deps); else if (sub === "context") action = () => context(argv, deps); else if (sub === "shadow") action = () => shadow(argv, deps); + else if (sub === "auto-review") action = () => autoReview(argv, deps); if (!action) return null; - return runCliAction(action); + let outcome: number | void = 0; + const exit = await runCliAction(async () => { outcome = await action(); }); + return exit || outcome || 0; } export const MODELS_RUNTIME_USAGE = USAGE; diff --git a/src/codex/catalog/auto-review.ts b/src/codex/catalog/auto-review.ts index be416776b87..dfb4b603ba2 100644 --- a/src/codex/catalog/auto-review.ts +++ b/src/codex/catalog/auto-review.ts @@ -480,20 +480,37 @@ function configHasProviderAutoReview(config: Pick): bool provider.autoReviewModel !== undefined || provider.autoReviewModelOverrides !== undefined); } +/** + * The global override selector, when the operator enabled it and named a model. + * + * Occupies the same slot as Codex's root `auto_review_model` fallback and wins over it: an + * enabled Model-settings choice is the operator's explicit pick, while a disabled or blank block + * leaves the root selector in charge. Provider-scoped stamps keep their precedence separately. + */ +export function resolveAutoReviewOverrideSelector( + config?: Pick, +): string | undefined { + const block = config?.autoReviewOverride; + if (block?.enabled !== true) return undefined; + const model = typeof block.model === "string" ? block.model.trim() : ""; + return model.length > 0 ? model : undefined; +} + /** Apply the root Codex auto-review selector after the final catalog merge. */ export function finalizeAutoReviewModelOverride( models: RawEntry[] | undefined, sourceModels: readonly RawEntry[] = [], - config?: Pick, + config?: Pick, ): AutoReviewModelOverrideResult { if (models && sourceModels.length > 0) preserveNativeAutoReviewModelOverrides( models.filter(entry => !isCodexControlPlaneModel(entry.slug)), sourceModels.filter(entry => !isCodexControlPlaneModel(entry.slug)), ); + const rootValue = resolveAutoReviewOverrideSelector(config) ?? readConfiguredAutoReviewModel(); if (config && configHasProviderAutoReview(config)) { - return applyConfiguredAutoReviewModelOverride(models, readConfiguredAutoReviewModel(), config, sourceModels); + return applyConfiguredAutoReviewModelOverride(models, rootValue, config, sourceModels); } - return applyAutoReviewModelOverride(models, readConfiguredAutoReviewModel(), sourceModels); + return applyAutoReviewModelOverride(models, rootValue, sourceModels); } /** * Why an account-gated native model stopped being offered, but only when the answer is one the diff --git a/src/config/load-degrade.ts b/src/config/load-degrade.ts index d994d7cedfd..b3887b3ffcf 100644 --- a/src/config/load-degrade.ts +++ b/src/config/load-degrade.ts @@ -8,6 +8,7 @@ import { anthropicSidecarConfigError } from "./schema/anthropic-account-pool"; import { modelPinnedEffortsConfigError, pinnedReasoningEffortConfigError, + autoReviewOverrideConfigError, autoReviewModelOverridesConfigError, autoReviewModelTargetConfigError, modelCapabilitiesConfigError, @@ -406,6 +407,11 @@ export function sanitizeModelCostsForLoad(parsed: unknown): void { export function sanitizeAutoReviewForLoad(parsed: unknown): void { if (!parsed || typeof parsed !== "object") return; const root = parsed as Record; + if (root.autoReviewOverride !== undefined + && autoReviewOverrideConfigError(root.autoReviewOverride) !== null) { + console.warn("⚠️ config.json autoReviewOverride is invalid — ignoring the override"); + delete root.autoReviewOverride; + } const providers = root.providers; if (!providers || typeof providers !== "object" || Array.isArray(providers)) return; for (const [name, providerValue] of Object.entries(providers as Record)) { diff --git a/src/config/provider-validation.ts b/src/config/provider-validation.ts index e7dc0eb0a7e..b33050ed080 100644 --- a/src/config/provider-validation.ts +++ b/src/config/provider-validation.ts @@ -256,10 +256,11 @@ export function autoReviewModelTargetConfigError( field = "autoReviewModel", allowClear = false, ): string | null { - if (value === undefined || (allowClear && (value === null || value === ""))) return null; + if (value === undefined || (allowClear && value === null)) return null; if (typeof value !== "string") return `${field} must be a string`; const trimmed = value.trim(); - if (!trimmed) return `${field} must be nonblank`; + // `allowClear` callers trim then delete, so whitespace-only is a clear, not a blank selector. + if (!trimmed) return allowClear ? null : `${field} must be nonblank`; if (trimmed.length > 1024 || AUTO_REVIEW_MODEL_CONTROL_CHARS.test(trimmed)) { return `${field} must be a catalog selector without whitespace or control characters`; } @@ -271,6 +272,26 @@ export function isValidAutoReviewModel(value: unknown): value is string { return typeof value === "string" && autoReviewModelTargetConfigError(value) === null; } +/** + * Validate the global auto-review override block the Models settings panel writes. + * + * `enabled` is optional so a save may carry only the model during an edit; `model` follows the + * same selector rules as the provider-scoped field, and an empty value clears the override. + */ +export function autoReviewOverrideConfigError(value: unknown, field = "autoReviewOverride"): string | null { + if (value === undefined || value === null) return null; + if (!value || typeof value !== "object" || Array.isArray(value)) return `${field} must be a plain object`; + const prototype = Object.getPrototypeOf(value); + if (prototype !== Object.prototype && prototype !== null) { + return `${field} must be a plain object with own properties`; + } + const block = value as Record; + if (block.enabled !== undefined && typeof block.enabled !== "boolean") { + return `${field}.enabled must be a boolean`; + } + return autoReviewModelTargetConfigError(block.model, `${field}.model`, true); +} + /** Canonical model key used for map matching, duplicate detection, and route tombstones. */ export function canonicalAutoReviewModelKey(modelId: string): string { return encodeRoutedModelId(modelId.trim()); diff --git a/src/config/schema/config-schema.ts b/src/config/schema/config-schema.ts index c90f4388676..04fb94e4e07 100644 --- a/src/config/schema/config-schema.ts +++ b/src/config/schema/config-schema.ts @@ -28,6 +28,7 @@ import { CODEX_ACCOUNT_NAMESPACE_ACCOUNT_ID_COLLISION_ERROR, codexAccountNamespacesSchema, modelPinnedEffortsSchema, + autoReviewOverrideSchema, compactionRoutingSchema, memoryModelSettingSchema, memoryModelsSchema, @@ -187,6 +188,9 @@ export const configSchema = z.object({ z.object({ enabled: z.literal(true), port: z.number().int().min(1).max(65535).optional() }), ]).optional().catch(undefined), providers: z.record(z.string(), providerConfigSchema), + // Models settings panel writes the global reviewer override; an invalid hand edit is + // stripped by load degradation so it cannot reject the providers that carry it. + autoReviewOverride: autoReviewOverrideSchema.optional(), modelPinnedEfforts: modelPinnedEffortsSchema.optional(), compactionRouting: compactionRoutingSchema.optional().catch(undefined), compactionRecovery: compactionRecoverySchema.optional().catch(undefined), diff --git a/src/config/schema/leaf-validators.ts b/src/config/schema/leaf-validators.ts index d9e1d07226e..cf606524b1a 100644 --- a/src/config/schema/leaf-validators.ts +++ b/src/config/schema/leaf-validators.ts @@ -6,6 +6,7 @@ import { modelPinnedEffortsConfigError, pinnedReasoningEffortConfigError, modelDisplayNamesConfigError, + autoReviewOverrideConfigError, autoReviewModelOverridesConfigError, autoReviewModelTargetConfigError, normalizeNonBlankStringArray, @@ -28,6 +29,7 @@ import { PROVIDER_WEB_SEARCH_BRIDGE_BACKENDS, UPSTREAM_HTTP_VERSION_VALUES, type OcxProviderConfig, + type OcxConfig, type FastWire, type ProviderCostOverlay, } from "../../types"; @@ -257,6 +259,12 @@ const autoReviewModelOverridesSchema = z.unknown().superRefine((value, ctx) => { if (error) ctx.addIssue({ code: "custom", message: error }); }).transform(value => normalizeAutoReviewModelOverrides(value)); +/** Global auto-review override block; a malformed hand edit is stripped by load degradation. */ +export const autoReviewOverrideSchema = z.unknown().superRefine((value, ctx) => { + const error = autoReviewOverrideConfigError(value, "autoReviewOverride"); + if (error) ctx.addIssue({ code: "custom", message: error }); +}).transform(value => value as NonNullable); + const modelCapabilitiesSchema = z.unknown().superRefine((value, ctx) => { const error = modelCapabilitiesConfigError(value); if (error) ctx.addIssue({ code: "custom", message: error }); diff --git a/src/server/management/config-routes.ts b/src/server/management/config-routes.ts index 1ea79b09d4a..b74809968e2 100644 --- a/src/server/management/config-routes.ts +++ b/src/server/management/config-routes.ts @@ -121,6 +121,8 @@ import { } from "../../lib/debug-settings"; import type { OcxClaudeCodeConfig, OcxConfig, OcxCustomModel, OcxProviderConfig } from "../../types"; import { shadowCallTargetError } from "./shadow-call-validation"; +import { autoReviewOverrideConfigError } from "../../config/provider-validation"; +import { ConfigWritePublishedError } from "../../config/persist-unlocked"; import { drainAndShutdown } from "../lifecycle"; import { filterRequestLogs, getRequestLogEntries, type RequestLogEntry } from "../request-log"; import { estimateComboCost, estimateRequestCost, normalizeCostTokens, tokensPerSecond } from "../../usage/cost"; @@ -1269,5 +1271,38 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise