From 8ecf91fc88d8dc07f429f590b74880f25f9f4a3a Mon Sep 17 00:00:00 2001 From: t Date: Mon, 7 Sep 2026 19:48:57 +0900 Subject: [PATCH 1/3] feat(gui): add guarded routed model picker drag ordering [skip ci] Add Custom routed ordering with fixed featured rows, keyboard controls, fresh settings snapshots and accepted-receipt recovery. Preserve native complete orders until an explicit preset/reset is accepted. Include source regressions and all nine locale strings; runtime checks await central CI. Implements the routed slice requested in #3774; native/featured full-picker placement remains open. Co-authored-by: leonclab <26379556+leonclab@users.noreply.github.com> --- .../src/content/docs/guides/model-ordering.md | 21 + gui/src/components/ModelPickerOrderEditor.tsx | 172 +++++++++ gui/src/i18n/de.ts | 15 + gui/src/i18n/en.ts | 15 + gui/src/i18n/fr.ts | 15 + gui/src/i18n/ja.ts | 15 + gui/src/i18n/ko.ts | 15 + gui/src/i18n/ru.ts | 15 + gui/src/i18n/tr.ts | 15 + gui/src/i18n/zh-TW.ts | 15 + gui/src/i18n/zh.ts | 15 + gui/src/model-picker-order.ts | 76 +++- gui/src/pages/Models.tsx | 61 ++- gui/src/styles-models-workspace.css | 7 + gui/tests/model-picker-order-editor.test.tsx | 358 ++++++++++++++++++ gui/tests/model-picker-order.test.ts | 109 +++++- 16 files changed, 917 insertions(+), 22 deletions(-) create mode 100644 gui/src/components/ModelPickerOrderEditor.tsx create mode 100644 gui/tests/model-picker-order-editor.test.tsx diff --git a/docs-site/src/content/docs/guides/model-ordering.md b/docs-site/src/content/docs/guides/model-ordering.md index 2d33b409d8..5333b839ce 100644 --- a/docs-site/src/content/docs/guides/model-ordering.md +++ b/docs-site/src/content/docs/guides/model-ordering.md @@ -178,3 +178,24 @@ On **Models**, choose **Default**, **A–Z by model**, **Group by provider**, or The controls use `GET/PUT /api/subagent-models`: `chosen` and `available` retain saved roster choices, including disabled or missing models; `pickerAvailable` contains only eligible routed catalog ids. The Models page sends `pickerOrder` and `pickerOrderMode`, never `models`. Roster-only saves preserve picker settings. Invalid combined updates and failed persistence leave the previous picker/roster state intact. Routed-only presets keep the existing featured/native priority bands. They affect the Codex catalog and Claude discovery's routed groups; Claude's native prefix and explicit Desktop profile/alias ownership remain unchanged. OpenCodex guidance ranks and configured fallback settings are preserved, but native Codex's advertised five and recommended default can change with display priority. Saving does not restart clients; a catalog refresh may remain pending, and clients holding an old catalog may need reopening. + + +### Custom routed order + +Choose **Custom order** on Models to load a fresh routed snapshot. Drag a movable row before +another row, or use its Up/Down buttons, then **Save draft**. Featured routed rows stay at the +front in their configured rank and cannot move. Native rows are not shown; this is not a preview +of the complete native picker. Surviving saved rows keep their relative order and new candidates +follow the current candidate list. Every save sends the complete routed list, without changing +the featured roster. + +An order containing bare native ids remains protected until you explicitly apply a routed preset +or Default. Selecting a different option alone does not replace it. Unknown featured state blocks +editing. Before saving, the editor checks a fresh snapshot; changes preserve your draft and block +saving until **Reload and discard draft** loads current settings. Request failures retain the +draft. Accepted saves can still have a pending catalog refresh; reload before editing again. + +The editor also requires an unambiguous model identity for every routed candidate. If the model +catalog is incomplete, refresh the Models page before editing; reloading picker settings alone +cannot restore missing catalog identities. Featured choices are matched exactly without trimming; +duplicate choices use their last configured position, and canonical ids take precedence over raw ids. diff --git a/gui/src/components/ModelPickerOrderEditor.tsx b/gui/src/components/ModelPickerOrderEditor.tsx new file mode 100644 index 0000000000..3c379f8cce --- /dev/null +++ b/gui/src/components/ModelPickerOrderEditor.tsx @@ -0,0 +1,172 @@ +import { useCallback, useEffect, useEffectEvent, useLayoutEffect, useRef, useState } from "react"; +import { createBoundedFetch, type BoundedFetch } from "../bounded-fetch"; +import { readJsonOrThrow } from "../fetch-json"; +import { IconArrowDown, IconArrowUp, IconGrip } from "../icons"; +import { useT, type TKey } from "../i18n/shared"; +import { + customPickerRows, isPickerOrderSaved, isPickerOrderSettings, movePickerBefore, + pickerSnapshotSignature, stepPickerOrder, type PickerModelIdentity, type PickerOrderSaved, +} from "../model-picker-order"; + +type Receipt = PickerOrderSaved & { catalogRefresh?: unknown }; +type Snapshot = { signature: string; identities: string; order: string[]; fixed: string[] }; +const DRAG_TYPE = "application/x-ocx-picker-order"; +let dragSequence = 0; +/** Local drag identity, not a security token. Like newClientId, supports LAN HTTP. */ +function newDragToken(): string { + const sequence = ++dragSequence; + if (typeof crypto !== "undefined" && typeof crypto.randomUUID === "function") { + try { return `${sequence}:${crypto.randomUUID()}`; } + catch { /* Some browsers expose randomUUID but reject it outside secure contexts. */ } + } + return `picker-${Date.now().toString(36)}-${sequence}`; +} + +export default function ModelPickerOrderEditor({ apiBase, active, identities, onAccepted, onBusyChange }: { + apiBase: string; active: boolean; identities: readonly PickerModelIdentity[]; + onAccepted: (receipt: Receipt) => void; onBusyChange: (busy: boolean) => void; +}) { + const t = useT(); + const [snapshot, setSnapshot] = useState(null); + const [draft, setDraft] = useState([]); + const [busy, setBusy] = useState(false); + const [blocked, setBlocked] = useState(null); + const [error, setError] = useState(false); + const [announcement, setAnnouncement] = useState(""); + const [dragging, setDragging] = useState(null); + const [over, setOver] = useState(null); + const drag = useRef<{ id: string; token: string } | null>(null); + const generation = useRef(0); + const flight = useRef(null); + const identitySignature = JSON.stringify(identities.map(({ provider, id, namespaced }) => [provider, id, namespaced])); + const latestIdentitySignature = useRef(identitySignature); + useLayoutEffect(() => { latestIdentitySignature.current = identitySignature; }, [identitySignature]); + const identityChanged = snapshot !== null && snapshot.identities !== identitySignature; + const disabled = !active || busy || !snapshot || blocked !== null || identityChanged; + const dirty = snapshot !== null && JSON.stringify(draft) !== JSON.stringify(snapshot.order); + const clearDrag = useCallback(() => { drag.current = null; setDragging(null); setOver(null); }, []); + + // Layout cleanup fences even A → B → A and unmount before a pending promise resumes. + useLayoutEffect(() => { + generation.current++; + setSnapshot(null); setDraft([]); setBlocked(null); setError(false); + return () => { + generation.current++; + flight.current?.controller.abort(); flight.current?.clear(); flight.current = null; + drag.current = null; onBusyChange(false); + }; + }, [apiBase, active, onBusyChange]); + useEffect(() => { clearDrag(); }, [disabled, snapshot, identitySignature, clearDrag]); + + const run = async (save: boolean) => { + if (!active || flight.current || (save && (disabled || !dirty))) return; + const owner = generation.current, bounded = createBoundedFetch(15_000); + flight.current = bounded; setBusy(true); onBusyChange(true); setError(false); clearDrag(); + const owns = () => generation.current === owner && flight.current === bounded; + const current = () => owns() && !bounded.signal.aborted + && latestIdentitySignature.current === identitySignature; + try { + const response = await fetch(`${apiBase}/api/subagent-models`, { signal: bounded.signal }); + if (!current()) return; + const settings = await readJsonOrThrow(response); + if (!current()) return; + if (!isPickerOrderSettings(settings)) throw new Error("Invalid picker settings"); + const signature = pickerSnapshotSignature(apiBase, owner, settings); + if (save && (!snapshot || signature !== snapshot.signature || identitySignature !== snapshot.identities)) { + setBlocked("models.pickerOrder.changed"); return; + } + const rows = customPickerRows(settings, identities); + if (!rows) { + setBlocked(settings.pickerOrder.some(id => !id.includes("/")) + ? "models.pickerOrder.nativeLocked" : settings.chosen === undefined + ? "models.pickerOrder.unknownChosen" : "models.pickerOrder.catalogRequired"); + return; + } + if (!save) { + setSnapshot({ ...rows, signature, identities: identitySignature }); setDraft(rows.order); + setBlocked(null); setAnnouncement(""); return; + } + const result = await fetch(`${apiBase}/api/subagent-models`, { + method: "PUT", headers: { "Content-Type": "application/json" }, signal: bounded.signal, + body: JSON.stringify({ pickerOrder: draft, pickerOrderMode: null }), + }); + if (!current()) return; + const receipt = await readJsonOrThrow(result); + if (!current()) return; + if (!isPickerOrderSaved(receipt) || !("ok" in receipt) || receipt.ok !== true) throw new Error("Invalid picker receipt"); + setDraft(receipt.pickerOrder); setBlocked("models.pickerOrder.savedReload"); + onAccepted({ pickerOrder: receipt.pickerOrder, pickerOrderMode: receipt.pickerOrderMode, + catalogRefresh: "catalogRefresh" in receipt ? receipt.catalogRefresh : undefined }); + } catch { + if (owns() && latestIdentitySignature.current === identitySignature) setError(true); + // Current-identity timeouts surface an error; stale identities retain the draft silently. + } finally { + bounded.clear(); + if (owns()) { flight.current = null; setBusy(false); onBusyChange(false); } + } + }; + const enter = useEffectEvent(() => { void run(false); }); + useEffect(() => { if (active) enter(); }, [apiBase, active]); + + const move = (id: string, next: string[]) => { + if (disabled) return; + setDraft(next); + setAnnouncement(t("models.pickerOrder.position", { model: id, position: next.indexOf(id) + 1, total: next.length })); + clearDrag(); + }; + const movable = (id: string) => !disabled && draft.includes(id) && !snapshot?.fixed.includes(id); + return
+

{t("models.pickerOrder.editorHint")}

+ {(blocked || identityChanged) &&

{t(blocked ?? "models.pickerOrder.changed")}

} + {error &&

{t("models.pickerOrder.requestFailed")}

} + {snapshot && draft.length === 0 &&

{t("models.pickerOrder.empty")}

} +
    + {draft.map((id, index) => { + const fixed = snapshot?.fixed.includes(id) === true; + return
  1. { + if (!drag.current || drag.current.id === id || !movable(drag.current.id) || !movable(id) + || !event.dataTransfer.types.includes(DRAG_TYPE)) return; + event.preventDefault(); event.dataTransfer.dropEffect = "move"; setOver(id); + }} + onDragLeave={() => setOver(null)} + onDrop={event => { + const source = drag.current; + if (source && source.id !== id && source.token === event.dataTransfer.getData(DRAG_TYPE) && movable(source.id) && movable(id)) { + event.preventDefault(); move(source.id, movePickerBefore(draft, source.id, id, snapshot?.fixed ?? [])); + } + clearDrag(); + }} onDragEnd={clearDrag}> + + {id} + {fixed && {t("models.pickerOrder.featured")}} + + + + +
    2. ; + })} +
+

{announcement}

+
+ + +
+
; +} diff --git a/gui/src/i18n/de.ts b/gui/src/i18n/de.ts index 79d1591728..d413e1277c 100644 --- a/gui/src/i18n/de.ts +++ b/gui/src/i18n/de.ts @@ -2625,4 +2625,19 @@ export const de: Record = { "usage.range.invalid": "Geben Sie gültige lokale Daten und Uhrzeiten ab 1970-01-01 UTC ein.", "usage.range.reversed": "Das Ende muss auf oder nach dem Beginn liegen.", "usage.range.applied": "Ausgewählter Zeitraum: {start} – {end} (beide Grenzen eingeschlossen).", + "models.pickerOrder.editorHint": "Routingsmodelle neu ordnen und den Entwurf speichern. Hervorgehobene Zeilen sind fest; native Modelle werden nicht angezeigt.", + "models.pickerOrder.nativeLocked": "Diese Reihenfolge enthält native Modelle. Vor der Bearbeitung eine Routing-Vorgabe oder Standard anwenden.", + "models.pickerOrder.unknownChosen": "Hervorgehobene Modelle sind unbekannt. Vor der Bearbeitung neu laden.", + "models.pickerOrder.changed": "Die Einstellungen haben sich geändert. Der Entwurf bleibt erhalten; erneutes Laden verwirft ihn und lädt die aktuellen Einstellungen.", + "models.pickerOrder.savedReload": "Reihenfolge gespeichert. Vor weiterer Bearbeitung aktuelle Einstellungen laden.", + "models.pickerOrder.requestFailed": "Anfrage fehlgeschlagen. Der Entwurf bleibt erhalten; erneut versuchen oder neu laden.", + "models.pickerOrder.empty": "Keine Routingmodelle verfügbar.", + "models.pickerOrder.dragModel": "{model} ziehen", + "models.pickerOrder.featured": "Hervorgehoben", + "models.pickerOrder.upModel": "{model} nach oben verschieben", + "models.pickerOrder.downModel": "{model} nach unten verschieben", + "models.pickerOrder.position": "{model}: Position {position} von {total}", + "models.pickerOrder.saveDraft": "Entwurf speichern", + "models.pickerOrder.reloadDraft": "Neu laden und Entwurf verwerfen", + "models.pickerOrder.catalogRequired": "Modellidentitäten fehlen oder sind mehrdeutig. Laden Sie die Modellseite neu, um den Katalog vor der Bearbeitung zu aktualisieren.", }; diff --git a/gui/src/i18n/en.ts b/gui/src/i18n/en.ts index cceef53d12..b842f4618c 100644 --- a/gui/src/i18n/en.ts +++ b/gui/src/i18n/en.ts @@ -2659,6 +2659,21 @@ export const en = { "usage.range.invalid": "Enter valid local dates and times, on or after 1970-01-01 UTC.", "usage.range.reversed": "The end must be at or after the start.", "usage.range.applied": "Selected interval: {start} – {end} (both inclusive).", + "models.pickerOrder.editorHint": "Reorder routed models, then save your draft. Featured rows are fixed; native models are not shown.", + "models.pickerOrder.nativeLocked": "This saved order includes native models. Apply a routed preset or Default before editing Custom.", + "models.pickerOrder.unknownChosen": "Featured choices are unknown. Reload before editing.", + "models.pickerOrder.changed": "Picker settings changed. Your draft is kept; reload to discard it and use current settings.", + "models.pickerOrder.savedReload": "Order saved. Reload current settings before editing again.", + "models.pickerOrder.requestFailed": "Request failed. Your draft is kept; retry or reload.", + "models.pickerOrder.empty": "No routed models are available.", + "models.pickerOrder.dragModel": "Drag {model}", + "models.pickerOrder.featured": "Featured", + "models.pickerOrder.upModel": "Move {model} up", + "models.pickerOrder.downModel": "Move {model} down", + "models.pickerOrder.position": "{model}: position {position} of {total}", + "models.pickerOrder.saveDraft": "Save draft", + "models.pickerOrder.reloadDraft": "Reload and discard draft", + "models.pickerOrder.catalogRequired": "Model identities are missing or ambiguous. Reload the Models page to refresh its catalog before editing Custom.", } as const; export type TKey = keyof typeof en; diff --git a/gui/src/i18n/fr.ts b/gui/src/i18n/fr.ts index 67b2586fe4..d07b7f7a16 100644 --- a/gui/src/i18n/fr.ts +++ b/gui/src/i18n/fr.ts @@ -2612,4 +2612,19 @@ export const fr: Record = { "usage.range.invalid": "Saisissez des dates et heures locales valides à partir du 1970-01-01 UTC.", "usage.range.reversed": "La fin doit être égale ou postérieure au début.", "usage.range.applied": "Période sélectionnée : {start} – {end} (bornes incluses).", + "models.pickerOrder.editorHint": "Réordonnez les modèles routés, puis enregistrez le brouillon. Les lignes mises en avant sont fixes ; les modèles natifs ne sont pas affichés.", + "models.pickerOrder.nativeLocked": "Cet ordre contient des modèles natifs. Appliquez un préréglage de routage ou Par défaut avant de le personnaliser.", + "models.pickerOrder.unknownChosen": "Les modèles mis en avant sont inconnus. Rechargez avant de modifier.", + "models.pickerOrder.changed": "Les paramètres ont changé. Le brouillon est conservé ; rechargez pour le supprimer et utiliser les paramètres actuels.", + "models.pickerOrder.savedReload": "Ordre enregistré. Rechargez les paramètres actuels avant de modifier à nouveau.", + "models.pickerOrder.requestFailed": "Échec de la requête. Le brouillon est conservé ; réessayez ou rechargez.", + "models.pickerOrder.empty": "Aucun modèle routé disponible.", + "models.pickerOrder.dragModel": "Faire glisser {model}", + "models.pickerOrder.featured": "Mis en avant", + "models.pickerOrder.upModel": "Monter {model}", + "models.pickerOrder.downModel": "Descendre {model}", + "models.pickerOrder.position": "{model} : position {position} sur {total}", + "models.pickerOrder.saveDraft": "Enregistrer le brouillon", + "models.pickerOrder.reloadDraft": "Recharger et supprimer le brouillon", + "models.pickerOrder.catalogRequired": "Les identités des modèles sont manquantes ou ambiguës. Rechargez la page Modèles pour actualiser le catalogue avant de personnaliser l’ordre.", }; diff --git a/gui/src/i18n/ja.ts b/gui/src/i18n/ja.ts index 92bb516827..1db94902c5 100644 --- a/gui/src/i18n/ja.ts +++ b/gui/src/i18n/ja.ts @@ -2646,4 +2646,19 @@ export const ja: Record = { "usage.range.invalid": "1970-01-01 UTC以降の有効な現地日時を入力してください。", "usage.range.reversed": "終了日時は開始日時と同じか、それ以降にしてください。", "usage.range.applied": "選択した期間：{start} – {end}（両端を含む）。", + "models.pickerOrder.editorHint": "ルーティングモデルを並べ替えて下書きを保存します。おすすめ行は固定され、ネイティブモデルは表示されません。", + "models.pickerOrder.nativeLocked": "保存済みの順序にネイティブモデルが含まれています。ルーティングのプリセットかデフォルトを適用してからカスタム順序を編集してください。", + "models.pickerOrder.unknownChosen": "おすすめモデルが不明です。再読み込みしてから編集してください。", + "models.pickerOrder.changed": "設定が変更されました。下書きは保持されます。再読み込みすると下書きを破棄し、現在の設定を使用します。", + "models.pickerOrder.savedReload": "順序を保存しました。再編集する前に現在の設定を読み込んでください。", + "models.pickerOrder.requestFailed": "リクエストに失敗しました。下書きは保持されます。再試行するか再読み込みしてください。", + "models.pickerOrder.empty": "利用可能なルーティングモデルはありません。", + "models.pickerOrder.dragModel": "{model} をドラッグ", + "models.pickerOrder.featured": "おすすめ", + "models.pickerOrder.upModel": "{model} を上へ移動", + "models.pickerOrder.downModel": "{model} を下へ移動", + "models.pickerOrder.position": "{model}: {total} 件中 {position} 番目", + "models.pickerOrder.saveDraft": "下書きを保存", + "models.pickerOrder.reloadDraft": "下書きを破棄して再読み込み", + "models.pickerOrder.catalogRequired": "モデルの識別情報が不足しているか曖昧です。モデルページを再読み込みしてカタログを更新してからカスタム順序を編集してください。", }; diff --git a/gui/src/i18n/ko.ts b/gui/src/i18n/ko.ts index 7f02d3792f..b31d6e4e1a 100644 --- a/gui/src/i18n/ko.ts +++ b/gui/src/i18n/ko.ts @@ -2647,4 +2647,19 @@ export const ko: Record = { "usage.range.invalid": "1970-01-01 UTC 이후의 유효한 현지 날짜와 시간을 입력하세요.", "usage.range.reversed": "종료 시각은 시작 시각과 같거나 이후여야 합니다.", "usage.range.applied": "선택한 기간: {start} – {end} (양 끝 시각 포함).", + "models.pickerOrder.editorHint": "라우팅 모델의 순서를 바꾼 뒤 초안을 저장하세요. 추천 모델은 고정되며 네이티브 모델은 표시하지 않습니다.", + "models.pickerOrder.nativeLocked": "저장된 순서에 네이티브 모델이 포함되어 있습니다. 라우팅 프리셋이나 기본값을 적용한 뒤 사용자 지정 순서를 편집하세요.", + "models.pickerOrder.unknownChosen": "추천 모델 정보를 확인할 수 없습니다. 다시 불러온 뒤 편집하세요.", + "models.pickerOrder.changed": "모델 선택 설정이 바뀌었습니다. 초안은 유지됩니다. 다시 불러오면 초안을 버리고 현재 설정을 사용합니다.", + "models.pickerOrder.savedReload": "순서가 저장되었습니다. 다시 편집하려면 현재 설정을 불러오세요.", + "models.pickerOrder.requestFailed": "요청에 실패했습니다. 초안은 유지됩니다. 재시도하거나 다시 불러오세요.", + "models.pickerOrder.empty": "사용 가능한 라우팅 모델이 없습니다.", + "models.pickerOrder.dragModel": "{model} 끌어서 이동", + "models.pickerOrder.featured": "추천 모델", + "models.pickerOrder.upModel": "{model} 위로 이동", + "models.pickerOrder.downModel": "{model} 아래로 이동", + "models.pickerOrder.position": "{model}: {total}개 중 {position}번째", + "models.pickerOrder.saveDraft": "초안 저장", + "models.pickerOrder.reloadDraft": "초안 버리고 다시 불러오기", + "models.pickerOrder.catalogRequired": "모델 식별 정보가 없거나 모호합니다. 모델 페이지를 새로고침해 목록을 갱신한 뒤 사용자 지정 순서를 편집하세요.", }; diff --git a/gui/src/i18n/ru.ts b/gui/src/i18n/ru.ts index 9548be7d64..a8370e7f5f 100644 --- a/gui/src/i18n/ru.ts +++ b/gui/src/i18n/ru.ts @@ -2648,4 +2648,19 @@ export const ru: Record = { "usage.range.invalid": "Введите допустимые местные дату и время не ранее 1970-01-01 UTC.", "usage.range.reversed": "Конец не может быть раньше начала.", "usage.range.applied": "Выбранный период: {start} – {end} (обе границы включены).", + "models.pickerOrder.editorHint": "Измените порядок маршрутизируемых моделей и сохраните черновик. Избранные строки закреплены; нативные модели не показаны.", + "models.pickerOrder.nativeLocked": "Сохранённый порядок содержит нативные модели. Перед редактированием примените пресет маршрутизации или порядок по умолчанию.", + "models.pickerOrder.unknownChosen": "Избранные модели неизвестны. Перезагрузите данные перед редактированием.", + "models.pickerOrder.changed": "Настройки изменились. Черновик сохранён; перезагрузка сбросит его и загрузит текущие настройки.", + "models.pickerOrder.savedReload": "Порядок сохранён. Перед следующим редактированием загрузите текущие настройки.", + "models.pickerOrder.requestFailed": "Ошибка запроса. Черновик сохранён; повторите запрос или перезагрузите данные.", + "models.pickerOrder.empty": "Нет доступных маршрутизируемых моделей.", + "models.pickerOrder.dragModel": "Перетащить {model}", + "models.pickerOrder.featured": "Избранная", + "models.pickerOrder.upModel": "Переместить {model} вверх", + "models.pickerOrder.downModel": "Переместить {model} вниз", + "models.pickerOrder.position": "{model}: позиция {position} из {total}", + "models.pickerOrder.saveDraft": "Сохранить черновик", + "models.pickerOrder.reloadDraft": "Перезагрузить и сбросить черновик", + "models.pickerOrder.catalogRequired": "Идентификаторы моделей отсутствуют или неоднозначны. Перезагрузите страницу моделей, чтобы обновить каталог перед редактированием порядка.", }; diff --git a/gui/src/i18n/tr.ts b/gui/src/i18n/tr.ts index 7b835eb03e..6a7c02c936 100644 --- a/gui/src/i18n/tr.ts +++ b/gui/src/i18n/tr.ts @@ -2648,4 +2648,19 @@ export const tr: Record = { "usage.range.invalid": "1970-01-01 UTC veya sonrasına ait geçerli yerel tarih ve saat girin.", "usage.range.reversed": "Bitiş, başlangıçla aynı veya daha sonra olmalıdır.", "usage.range.applied": "Seçilen aralık: {start} – {end} (iki sınır da dahil).", + "models.pickerOrder.editorHint": "Yönlendirilen modelleri sıralayıp taslağı kaydedin. Öne çıkan satırlar sabittir; yerel modeller gösterilmez.", + "models.pickerOrder.nativeLocked": "Kayıtlı sıra yerel modeller içeriyor. Özel sırayı düzenlemeden önce yönlendirme ön ayarını veya Varsayılan seçeneğini uygulayın.", + "models.pickerOrder.unknownChosen": "Öne çıkan modeller bilinmiyor. Düzenlemeden önce yeniden yükleyin.", + "models.pickerOrder.changed": "Seçici ayarları değişti. Taslağınız korunuyor; yeniden yüklemek taslağı siler ve güncel ayarları kullanır.", + "models.pickerOrder.savedReload": "Sıra kaydedildi. Yeniden düzenlemeden önce güncel ayarları yükleyin.", + "models.pickerOrder.requestFailed": "İstek başarısız. Taslağınız korunuyor; tekrar deneyin veya yeniden yükleyin.", + "models.pickerOrder.empty": "Kullanılabilir yönlendirilen model yok.", + "models.pickerOrder.dragModel": "{model} modelini sürükle", + "models.pickerOrder.featured": "Öne çıkan", + "models.pickerOrder.upModel": "{model} modelini yukarı taşı", + "models.pickerOrder.downModel": "{model} modelini aşağı taşı", + "models.pickerOrder.position": "{model}: {total} içinde {position}. sıra", + "models.pickerOrder.saveDraft": "Taslağı kaydet", + "models.pickerOrder.reloadDraft": "Yeniden yükle ve taslağı sil", + "models.pickerOrder.catalogRequired": "Model kimlikleri eksik veya belirsiz. Özel sırayı düzenlemeden önce kataloğu yenilemek için Modeller sayfasını yeniden yükleyin.", }; diff --git a/gui/src/i18n/zh-TW.ts b/gui/src/i18n/zh-TW.ts index cd0b7e17d3..d59fa6d6bc 100644 --- a/gui/src/i18n/zh-TW.ts +++ b/gui/src/i18n/zh-TW.ts @@ -2610,4 +2610,19 @@ export const zhTW: Record = { "usage.range.invalid": "請輸入不早於 1970-01-01 UTC 的有效本地日期和時間。", "usage.range.reversed": "結束時間必須等於或晚於開始時間。", "usage.range.applied": "所選範圍：{start} – {end}（包含兩端）。", + "models.pickerOrder.editorHint": "調整路由模型順序後儲存草稿。精選列固定，原生模型不在此顯示。", + "models.pickerOrder.nativeLocked": "已儲存的順序包含原生模型。請先套用路由預設或預設順序，再編輯自訂順序。", + "models.pickerOrder.unknownChosen": "精選模型資訊未知。請重新載入後再編輯。", + "models.pickerOrder.changed": "模型選擇設定已變更。草稿已保留；重新載入將捨棄草稿並使用目前設定。", + "models.pickerOrder.savedReload": "順序已儲存。再次編輯前請重新載入目前設定。", + "models.pickerOrder.requestFailed": "請求失敗。草稿已保留；請重試或重新載入。", + "models.pickerOrder.empty": "沒有可用的路由模型。", + "models.pickerOrder.dragModel": "拖曳 {model}", + "models.pickerOrder.featured": "精選", + "models.pickerOrder.upModel": "上移 {model}", + "models.pickerOrder.downModel": "下移 {model}", + "models.pickerOrder.position": "{model}：第 {position} 位，共 {total} 個", + "models.pickerOrder.saveDraft": "儲存草稿", + "models.pickerOrder.reloadDraft": "捨棄草稿並重新載入", + "models.pickerOrder.catalogRequired": "模型識別資訊缺失或不明確。請重新載入模型頁面以更新目錄，再編輯自訂順序。", }; diff --git a/gui/src/i18n/zh.ts b/gui/src/i18n/zh.ts index d9f93f20c7..d20318ce42 100644 --- a/gui/src/i18n/zh.ts +++ b/gui/src/i18n/zh.ts @@ -2646,4 +2646,19 @@ export const zh: Record = { "usage.range.invalid": "请输入不早于 1970-01-01 UTC 的有效本地日期和时间。", "usage.range.reversed": "结束时间必须等于或晚于开始时间。", "usage.range.applied": "所选范围：{start} – {end}（包含两端）。", + "models.pickerOrder.editorHint": "调整路由模型顺序后保存草稿。精选行固定，原生模型不在此显示。", + "models.pickerOrder.nativeLocked": "已保存的顺序包含原生模型。请先应用路由预设或默认顺序，再编辑自定义顺序。", + "models.pickerOrder.unknownChosen": "精选模型信息未知。请重新加载后再编辑。", + "models.pickerOrder.changed": "模型选择设置已更改。草稿已保留；重新加载将丢弃草稿并使用当前设置。", + "models.pickerOrder.savedReload": "顺序已保存。再次编辑前请重新加载当前设置。", + "models.pickerOrder.requestFailed": "请求失败。草稿已保留；请重试或重新加载。", + "models.pickerOrder.empty": "没有可用的路由模型。", + "models.pickerOrder.dragModel": "拖动 {model}", + "models.pickerOrder.featured": "精选", + "models.pickerOrder.upModel": "上移 {model}", + "models.pickerOrder.downModel": "下移 {model}", + "models.pickerOrder.position": "{model}：第 {position} 位，共 {total} 个", + "models.pickerOrder.saveDraft": "保存草稿", + "models.pickerOrder.reloadDraft": "丢弃草稿并重新加载", + "models.pickerOrder.catalogRequired": "模型标识信息缺失或不明确。请重新加载模型页面以刷新目录，再编辑自定义顺序。", }; diff --git a/gui/src/model-picker-order.ts b/gui/src/model-picker-order.ts index dfa5073b08..0de6190f02 100644 --- a/gui/src/model-picker-order.ts +++ b/gui/src/model-picker-order.ts @@ -11,7 +11,7 @@ export interface PickerOrderSaved { pickerOrder: string[]; pickerOrderMode: SavedModelPickerOrderMode | null; } -export interface PickerOrderSettings extends PickerOrderSaved { pickerAvailable: string[] } +export interface PickerOrderSettings extends PickerOrderSaved { pickerAvailable: string[]; chosen?: string[] } function stringList(value: unknown): value is string[] { return Array.isArray(value) && value.every(id => typeof id === "string" && id.trim().length > 0); @@ -25,7 +25,11 @@ export function isPickerOrderSaved(value: unknown): value is PickerOrderSaved { return stringList(row.pickerOrder) && savedMode(row.pickerOrderMode); } export function isPickerOrderSettings(value: unknown): value is PickerOrderSettings { - return isPickerOrderSaved(value) && stringList((value as PickerOrderSettings).pickerAvailable); + if (!isPickerOrderSaved(value)) return false; + const row = value as PickerOrderSettings; + // Roster writes accept every string, including blanks; picker fields remain nonempty-string lists. + return stringList(row.pickerAvailable) && (!("chosen" in row) + || (Array.isArray(row.chosen) && row.chosen.every(id => typeof id === "string"))); } export function isModelPickerUsage(value: unknown): value is ModelPickerUsage[] { return Array.isArray(value) && value.every(row => row !== null && typeof row === "object" @@ -104,3 +108,71 @@ export function modelPickerOrderMode( } return "custom"; } + + +/** Resolve exact canonical ids before legacy provider/raw spellings; never guess a bare native id. */ +export function normalizePickerIds(ids: readonly string[], available: readonly string[], identities: readonly PickerModelIdentity[]): string[] { + const candidates = new Set(available.filter(id => id.includes("/"))); + const resolve = (id: string): string | undefined => { + if (candidates.has(id)) return id; + const matches = new Set(identities.filter(row => candidates.has(row.namespaced) + && id === `${row.provider}/${row.id}`).map(row => row.namespaced)); + return matches.size === 1 ? [...matches][0] : undefined; + }; + return [...new Set(ids.map(id => resolve(id.trim())).filter((id): id is string => id !== undefined))]; +} + +export function pickerSnapshotSignature(apiBase: string, generation: number, settings: PickerOrderSettings): string { + return JSON.stringify([apiBase, generation, settings.pickerAvailable, settings.chosen ?? null, + settings.pickerOrder, settings.pickerOrderMode]); +} + +/** Every candidate needs one observed provider/raw identity, with no encoded/raw collisions. */ +export function pickerIdentityCoverage(available: readonly string[], identities: readonly PickerModelIdentity[]): boolean { + const candidates = new Set(available.filter(id => id.includes("/"))); + const rawBySlug = new Map>(), slugsByRaw = new Map>(); + for (const row of identities) { + if (!candidates.has(row.namespaced)) continue; + const raw = `${row.provider}/${row.id}`; + const raws = rawBySlug.get(row.namespaced) ?? new Set(); + const slugs = slugsByRaw.get(raw) ?? new Set(); + raws.add(raw); slugs.add(row.namespaced); + rawBySlug.set(row.namespaced, raws); slugsByRaw.set(raw, slugs); + } + return [...candidates].every(slug => { + const raws = rawBySlug.get(slug); + return raws?.size === 1 && slugsByRaw.get([...raws][0]!)?.size === 1; + }); +} + +export function customPickerRows(settings: PickerOrderSettings, identities: readonly PickerModelIdentity[]): { order: string[]; fixed: string[] } | null { + // Unknown featured state and complete/native orders cannot safely become routed-only drafts. + if (settings.chosen === undefined || settings.pickerOrder.some(id => !id.includes("/"))) return null; + const available = [...new Set(settings.pickerAvailable.filter(id => id.includes("/")))]; + if (!pickerIdentityCoverage(available, identities)) return null; + // Roster strings stay verbatim. Map uses the LAST occurrence; each row prefers its exact canonical rank. + const chosenRank = new Map(settings.chosen.map((id, index) => [id, index])); + const rawBySlug = new Map(identities.map(row => [row.namespaced, `${row.provider}/${row.id}`])); + const rankOf = (slug: string) => chosenRank.get(slug) ?? chosenRank.get(rawBySlug.get(slug)!); + const fixed = available.filter(slug => rankOf(slug) !== undefined).sort((a, b) => rankOf(a)! - rankOf(b)!); + const saved = normalizePickerIds(settings.pickerOrder, available, identities); + return { fixed, order: [...new Set([...fixed, ...saved, ...available])] }; +} + +/** Drop semantics: remove first, re-find the target, then insert before it. */ +export function movePickerBefore(order: readonly string[], source: string, target: string, fixed: readonly string[]): string[] { + const next = [...order]; + if (source === target || fixed.includes(source) || fixed.includes(target) + || !next.includes(source) || !next.includes(target)) return next; + next.splice(next.indexOf(source), 1); + next.splice(next.indexOf(target), 0, source); + return next; +} + +/** Keyboard semantics deliberately differ from dropping before the next row. */ +export function stepPickerOrder(order: readonly string[], source: string, direction: -1 | 1, fixed: readonly string[]): string[] { + const next = [...order], index = next.indexOf(source), target = index + direction; + if (index < 0 || target < 0 || target >= next.length || fixed.includes(source) || fixed.includes(next[target]!)) return next; + [next[index], next[target]] = [next[target]!, next[index]!]; + return next; +} diff --git a/gui/src/pages/Models.tsx b/gui/src/pages/Models.tsx index bc010c4abc..55d9c16106 100644 --- a/gui/src/pages/Models.tsx +++ b/gui/src/pages/Models.tsx @@ -1,10 +1,11 @@ import { CodexStaleBanner } from "../components/codex-stale-banner"; +import ModelPickerOrderEditor from "../components/ModelPickerOrderEditor"; import ModelDisplayNameDialog from "../components/ModelDisplayNameDialog"; import ModelPriceDialog from "../components/ModelPriceDialog"; import { fetchCodexAppServerState } from "../codex-app-server-state"; import type { AppServerStateOutcome } from "../codex-app-server-state"; import { useCodexRestart } from "../use-codex-restart"; -import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from "react"; import { Switch, Notice, EmptyState, Select, Tooltip } from "../ui"; import { IconChevron, IconBoxes, IconInfo, IconCheck, IconAlert, IconRefresh, IconPencil } from "../icons"; import { useT } from "../i18n/shared"; @@ -18,7 +19,7 @@ import { setClientResourceData } from "../client-resource"; import { createBoundedFetch, type BoundedFetch } from "../bounded-fetch"; import { isModelPickerUsage, isPickerOrderSaved, isPickerOrderSettings, modelPickerOrder, modelPickerOrderMode, - type ModelPickerOrderMode, type PickerOrderSettings, type ModelPickerUsage, + type ModelPickerOrderMode, type PickerOrderSettings, type PickerOrderSaved, type ModelPickerUsage, } from "../model-picker-order"; import { startVisibilityPoll } from "../visibility-poll"; import { useDataSurface } from "../data-surface"; @@ -253,6 +254,7 @@ export default function Models({ apiBase, restartEpoch = 0 }: { apiBase: string; const [pickerDraft, setPickerDraft] = useState(null); const [pickerBusy, setPickerBusy] = useState(false); const pickerFlight = useRef(null); + const pickerGeneration = useRef(0); const pickerResource = useDataSurface( pickerCacheKey, [apiBase], useCallback(async (signal: AbortSignal) => { @@ -269,16 +271,22 @@ export default function Models({ apiBase, restartEpoch = 0 }: { apiBase: string; const pickerMode = pickerDraft ?? modelPickerOrderMode( pickerSettings?.pickerAvailable ?? [], pickerSettings?.pickerOrder ?? [], pickerSettings?.pickerOrderMode, ); - useEffect(() => { + useLayoutEffect(() => { + pickerGeneration.current++; setPickerDraft(null); setPickerBusy(false); return () => { + pickerGeneration.current++; pickerFlight.current?.controller.abort(); pickerFlight.current?.clear(); pickerFlight.current = null; cancelAppServerRead(); }; }, [apiBase, catalogActive, cancelAppServerRead]); + useLayoutEffect(() => { + // Pin inferred Custom before any late GET can switch mode and unmount its draft. + if (catalogActive && pickerDraft === null && pickerMode === "custom") setPickerDraft("custom"); + }, [catalogActive, pickerDraft, pickerMode]); const [customCap, setCustomCap] = useState(""); const [showCustom, setShowCustom] = useState(false); const [providerCapCustomOpen, setProviderCapCustomOpen] = useState>({}); @@ -1825,49 +1833,61 @@ export default function Models({ apiBase, restartEpoch = 0 }: { apiBase: string; ? groups.filter(group => group.provider === selectedProvider) : groups; + const acceptPickerOrder = (data: PickerOrderSaved & { catalogRefresh?: unknown }, custom = false) => { + // A receipt proves only the saved fields. No old chosen/available snapshot is promoted. + const next: PickerOrderSettings = { pickerOrder: data.pickerOrder, pickerOrderMode: data.pickerOrderMode, pickerAvailable: [] }; + setClientResourceData(pickerCacheKey, next); + writeSessionListCache(pickerCacheKey, next); + if (custom) setPickerDraft("custom"); + pickerResource.refresh(); + const refresh = data.catalogRefresh; + const converged = refresh !== null && typeof refresh === "object" + && "status" in refresh && refresh.status === "committed" + && "degraded" in refresh && refresh.degraded === false; + publishFeedback(converged, t(converged ? "models.pickerOrder.saved" : "models.pickerOrder.pending")); + void reloadAppServerState(); + }; + const savePickerOrder = async () => { if (pickerFlight.current || !pickerSettings || pickerResource.state.showError || pickerMode === "custom") return; + const owner = pickerGeneration.current; const mode = pickerMode; const available = pickerSettings.pickerAvailable; const bounded = createBoundedFetch(15_000); pickerFlight.current = bounded; setPickerBusy(true); + const owns = () => pickerGeneration.current === owner && pickerFlight.current === bounded; + const current = () => owns() && !bounded.signal.aborted; try { let usage: ModelPickerUsage[] = []; if (mode === "most-used") { const response = await fetch(`${apiBase}/api/usage?range=all&surface=all`, { signal: bounded.signal }); + if (!current()) return; const payload = await readJsonOrThrow<{ models?: unknown }>(response, t("models.pickerOrder.usageFailed")); + if (!current()) return; if (!isModelPickerUsage(payload?.models)) throw new Error(t("models.pickerOrder.usageFailed")); usage = payload.models; } + if (!current()) return; const order = modelPickerOrder(mode, available, usage, models); const response = await fetch(`${apiBase}/api/subagent-models`, { method: "PUT", headers: { "Content-Type": "application/json" }, signal: bounded.signal, body: JSON.stringify({ pickerOrder: order, pickerOrderMode: mode === "default" ? null : mode }), }); + if (!current()) return; const data = await readJsonOrThrow(response, t("models.saveFailed")); if (!isPickerOrderSaved(data) || !("ok" in data) || data.ok !== true) throw new Error(t("models.saveFailed")); - if (bounded.signal.aborted || pickerFlight.current !== bounded) return; - const next = { ...pickerSettings, pickerOrder: data.pickerOrder, pickerOrderMode: data.pickerOrderMode }; - // This aborts an older GET and advances the shared resource generation. - setClientResourceData(pickerCacheKey, next); - writeSessionListCache(pickerCacheKey, next); + if (!current()) return; + acceptPickerOrder({ pickerOrder: data.pickerOrder, pickerOrderMode: data.pickerOrderMode, + catalogRefresh: "catalogRefresh" in data ? data.catalogRefresh : undefined }); setPickerDraft(null); - - const refresh = "catalogRefresh" in data ? data.catalogRefresh : undefined; - const converged = refresh !== null && typeof refresh === "object" - && "status" in refresh && refresh.status === "committed" - && "degraded" in refresh && refresh.degraded === false; - publishFeedback(converged, t(converged ? "models.pickerOrder.saved" : "models.pickerOrder.pending")); - // Durable save is already accepted. Observational failure must not undo it. - void reloadAppServerState(); } catch (error) { - if (pickerFlight.current === bounded) { + if (owns()) { publishFeedback(false, error instanceof Error ? error.message : t("models.networkError")); } } finally { bounded.clear(); - if (pickerFlight.current === bounded) { pickerFlight.current = null; setPickerBusy(false); } + if (owns()) { pickerFlight.current = null; setPickerBusy(false); } } }; @@ -2039,7 +2059,7 @@ export default function Models({ apiBase, restartEpoch = 0 }: { apiBase: string; { value: "alphabetical", label: t("models.pickerOrder.alphabetical") }, { value: "provider", label: t("models.pickerOrder.provider") }, { value: "most-used", label: t("models.pickerOrder.mostUsed") }, - ...(pickerMode === "custom" ? [{ value: "custom", label: t("models.pickerOrder.custom") }] : []), + { value: "custom", label: t("models.pickerOrder.custom") }, ]} onChange={value => setPickerDraft(value as ModelPickerOrderMode)} disabled={pickerBusy || !pickerSettings || pickerResource.state.showError} @@ -2058,6 +2078,9 @@ export default function Models({ apiBase, restartEpoch = 0 }: { apiBase: string; } {t("models.pickerOrder.hint")} + {pickerMode === "custom" && acceptPickerOrder(data, true)} />} + {(() => { const customCount = models.filter(m => m.custom).length; diff --git a/gui/src/styles-models-workspace.css b/gui/src/styles-models-workspace.css index 6195a7b24f..67872711e2 100644 --- a/gui/src/styles-models-workspace.css +++ b/gui/src/styles-models-workspace.css @@ -648,3 +648,10 @@ } } .models-integration-warning { overflow-wrap: anywhere; } + +.picker-order-editor { margin-block: 12px; } +.picker-order-list { list-style: none; margin: 0; padding: 0; display: flex; flex-direction: column; gap: 8px; } +.picker-order-row { display: flex; align-items: center; flex-wrap: wrap; gap: 8px; padding-block: 4px; } +.picker-order-name { flex: 1; min-width: 0; overflow-wrap: anywhere; } +.picker-order-actions { display: inline-flex; flex-shrink: 0; gap: 2px; } +.picker-order-row .cwi-target-grip:disabled { cursor: default; opacity: 0.5; } diff --git a/gui/tests/model-picker-order-editor.test.tsx b/gui/tests/model-picker-order-editor.test.tsx new file mode 100644 index 0000000000..7cfeb6b326 --- /dev/null +++ b/gui/tests/model-picker-order-editor.test.tsx @@ -0,0 +1,358 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { Window } from "happy-dom"; +import { act } from "react"; +import type { Root } from "react-dom/client"; +import Models from "../src/pages/Models"; +import { clearClientResourceStoresForTests, setClientResourceData } from "../src/client-resource"; +import ModelPickerOrderEditor from "../src/components/ModelPickerOrderEditor"; +import { LanguageProvider } from "../src/i18n/provider"; +import type { PickerModelIdentity, PickerOrderSettings, PickerOrderSaved } from "../src/model-picker-order"; + +const globals = ["document", "window", "navigator", "localStorage", "sessionStorage", "fetch", "crypto", "IS_REACT_ACT_ENVIRONMENT"] as const; +const ids: PickerModelIdentity[] = ["f", "a", "b", "c"].map(id => ({ provider: "p", id, namespaced: `p/${id}` })); +const initial = (): PickerOrderSettings => ({ pickerAvailable: ["p/f", "p/a", "p/b", "p/c"], + chosen: ["native", "p/f"], pickerOrder: ["p/a", "p/b", "p/c", "p/f"], pickerOrderMode: null }); +const changedDraft = ["p/f", "p/b", "p/a", "p/c"]; +function deferred() { + let resolve!: (value: T) => void, reject!: (error: Error) => void; + const promise = new Promise((yes, no) => { resolve = yes; reject = no; }); + return { promise, resolve, reject }; +} +type Request = ReturnType> & { url: string; method: string; body: unknown; signal?: AbortSignal | null }; +let previous: Map; +let win: Window, host: HTMLElement, root: Root | null; +let requests: Request[], receipts: Array, busy: boolean[]; +const onAccepted = (value: PickerOrderSaved & { catalogRefresh?: unknown }) => { receipts.push(value); }; +const onBusyChange = (value: boolean) => { busy.push(value); }; + +beforeEach(() => { + clearClientResourceStoresForTests(); + previous = new Map(globals.map(key => [key, Object.getOwnPropertyDescriptor(globalThis, key)])); + win = new Window({ url: "http://localhost/#models" }); + win.localStorage.setItem("ocx-lang", "en"); + const values = { document: win.document, window: win, navigator: win.navigator, + localStorage: win.localStorage, sessionStorage: win.sessionStorage, IS_REACT_ACT_ENVIRONMENT: true }; + for (const [key, value] of Object.entries(values)) Object.defineProperty(globalThis, key, { configurable: true, value }); + requests = []; receipts = []; busy = []; root = null; + Object.defineProperty(globalThis, "fetch", { configurable: true, value: (input: RequestInfo | URL, init?: RequestInit) => { + // Intentionally ignores abort: late network/body completion must be fenced by the component. + const request = { ...deferred(), url: String(input), method: init?.method ?? "GET", + body: init?.body ? JSON.parse(String(init.body)) : undefined, signal: init?.signal }; + requests.push(request); return request.promise; + } }); + host = document.createElement("div"); document.body.append(host); +}); +afterEach(async () => { + if (root) await act(async () => { root!.unmount(); }); + clearClientResourceStoresForTests(); + win.close(); + for (const key of globals) { + const descriptor = previous.get(key); + if (descriptor) Object.defineProperty(globalThis, key, descriptor); + else Reflect.deleteProperty(globalThis, key); + } +}); +async function render(apiBase = "/a", identities = ids, active = true) { + const { createRoot } = await import("react-dom/client"); + await act(async () => { + root ??= createRoot(host); + root.render(); + }); +} +async function reply(index: number, data: unknown, status = 200) { + await act(async () => { requests[index]!.resolve(Response.json(data, { status })); }); +} +const order = (within: ParentNode = host) => [...within.querySelectorAll(".picker-order-name")].map(row => row.textContent); +function button(name: string, within: ParentNode = host): HTMLButtonElement { + const found = [...within.querySelectorAll("button")] + .find(node => node.getAttribute("aria-label") === name || node.textContent === name); + if (!found) throw new Error(`Missing button: ${name}`); + return found; +} +async function click(name: string) { await act(async () => { button(name).click(); }); } +function row(id: string, within: ParentNode = host): HTMLElement { + const found = [...within.querySelectorAll("li")].find(node => node.querySelector("code")?.textContent === id); + if (!found) throw new Error(`Missing row: ${id}`); + return found; +} +function transfer() { + const data = new Map(); + return { effectAllowed: "uninitialized", dropEffect: "none", get types() { return [...data.keys()]; }, + setData: (type: string, value: string) => { data.set(type, value); }, getData: (type: string) => data.get(type) ?? "" }; +} +async function dragEvent(target: Element, type: string, dataTransfer: ReturnType) { + await act(async () => { + const event = new win.Event(type, { bubbles: true, cancelable: true }); + Object.defineProperty(event, "dataTransfer", { value: dataTransfer }); target.dispatchEvent(event); + }); +} +async function drop(source: string, target: string) { + const data = transfer(); + await dragEvent(button(`Drag ${source}`), "dragstart", data); + await dragEvent(row(target), "dragover", data); + await dragEvent(row(target), "drop", data); +} +async function edit() { await render(); await reply(0, initial()); await click("Move p/a down"); } + +// No sleeps, retries or real transport: each deferred settlement is explicitly released in act. +test("entering Custom reads a fresh GET each activation and only renders pickerAvailable", async () => { + await render("/a", ids, false); expect(requests).toHaveLength(0); + await render(); expect(requests.map(r => [r.url, r.method])).toEqual([["/a/api/subagent-models", "GET"]]); + expect(order()).toEqual([]); expect(busy.at(-1)).toBe(true); + await reply(0, { ...initial(), available: ["native", "other/roster-only"] }); + expect(order()).toEqual(["p/f", "p/a", "p/b", "p/c"]); expect(busy.at(-1)).toBe(false); + await render("/a", ids, false); await render(); expect(requests).toHaveLength(2); + await reply(1, { ...initial(), pickerOrder: ["p/c", "p/b", "p/a"] }); + expect(order()).toEqual(["p/f", "p/c", "p/b", "p/a"]); +}); + +for (const [name, override] of [ + ["missing", {}], ["null", { chosen: null }], ["non-array", { chosen: "p/f" }], ["invalid item", { chosen: [1] }], +] as const) test(`Custom cannot edit with ${name} chosen`, async () => { + await render(); + const { chosen: _chosen, ...settings } = initial(); + await reply(0, { ...settings, ...override }); + expect(order()).toEqual([]); expect(host.querySelector('[role="alert"]')).not.toBeNull(); + expect(button("Save draft").disabled).toBe(true); + await click("Save draft"); expect(requests).toHaveLength(1); +}); +test("saved bare native order remains locked without sending a replacement", async () => { + await render(); await reply(0, { ...initial(), pickerOrder: ["native", "p/a"] }); + expect(host.textContent).toContain("This saved order includes native models."); + expect(button("Save draft").disabled).toBe(true); expect(receipts).toEqual([]); + expect(requests.map(r => r.method)).toEqual(["GET"]); +}); + +test("forward/backward drop and Up/Down controls submit the complete routed list only", async () => { + await render(); await reply(0, initial()); + expect(button("Move p/f down").disabled).toBe(true); expect(button("Move p/a up").disabled).toBe(true); + await drop("p/a", "p/c"); expect(order()).toEqual(["p/f", "p/b", "p/a", "p/c"]); + await drop("p/c", "p/b"); expect(order()).toEqual(["p/f", "p/c", "p/b", "p/a"]); + button("Move p/c down").focus(); await click("Move p/c down"); + expect(order()).toEqual(["p/f", "p/b", "p/c", "p/a"]); + expect(document.activeElement).toBe(button("Move p/c down")); + await click("Move p/a up"); expect(order()).toEqual(changedDraft); + expect(host.querySelector('[role="status"]')?.textContent).toBe("p/a: position 3 of 4"); + await click("Save draft"); expect(requests.map(r => r.method)).toEqual(["GET", "GET"]); + await reply(1, initial()); + expect(requests[2]?.method).toBe("PUT"); + expect(requests[2]?.body).toEqual({ pickerOrder: changedDraft, pickerOrderMode: null }); +}); + +test("external, self, fixed and expired drag tokens cannot reorder", async () => { + await render(); await reply(0, initial()); + const original = ["p/f", "p/a", "p/b", "p/c"], external = transfer(); + external.setData("application/x-ocx-picker-order", "external"); + await dragEvent(row("p/b"), "drop", external); expect(order()).toEqual(original); + await drop("p/a", "p/a"); await drop("p/a", "p/f"); expect(order()).toEqual(original); + const local = transfer(); await dragEvent(button("Drag p/a"), "dragstart", local); + await dragEvent(row("p/b"), "drop", external); expect(order()).toEqual(original); + await dragEvent(row("p/b"), "drop", local); expect(order()).toEqual(original); + await dragEvent(button("Drag p/a"), "dragstart", local); + await dragEvent(row("p/a"), "dragend", local); + await dragEvent(row("p/c"), "drop", local); expect(order()).toEqual(original); +}); + +test("preflight roster drift blocks PUT, preserves draft, and requires explicit reload", async () => { + await edit(); await click("Save draft"); + const updated = { ...initial(), chosen: ["p/b"] }; + await reply(1, updated); + expect(order()).toEqual(changedDraft); expect(button("Save draft").disabled).toBe(true); + expect(host.textContent).toContain("Picker settings changed."); + await click("Save draft"); expect(requests.map(r => r.method)).toEqual(["GET", "GET"]); + await click("Reload and discard draft"); expect(order()).toEqual(changedDraft); + await reply(2, updated); expect(order()).toEqual(["p/b", "p/a", "p/c", "p/f"]); + expect(button("Move p/a down").disabled).toBe(false); expect(receipts).toEqual([]); +}); + +for (const failure of ["rejected", "malformed JSON", "malformed receipt", "network"] as const) + test(`failed PUT (${failure}) retains draft for a fresh preflight retry`, async () => { + await edit(); await click("Save draft"); await reply(1, initial()); + if (failure === "network") await act(async () => { requests[2]!.reject(new Error("offline")); }); + else if (failure === "malformed JSON") await act(async () => { requests[2]!.resolve(new Response("{")); }); + else await reply(2, failure === "rejected" ? { error: "refused" } : { ok: true, pickerOrder: [] }, failure === "rejected" ? 409 : 200); + expect(order()).toEqual(changedDraft); expect(receipts).toEqual([]); + expect(host.textContent).toContain("Request failed. Your draft is kept;"); + expect(button("Save draft").disabled).toBe(false); + await click("Save draft"); expect(requests[3]?.method).toBe("GET"); + await reply(3, initial()); expect(requests[4]?.body).toEqual({ pickerOrder: changedDraft, pickerOrderMode: null }); + }); + +test("pending accepted receipt publishes saved fields and requires reload before editing again", async () => { + await edit(); await click("Save draft"); await reply(1, initial()); + const accepted = { pickerOrder: changedDraft, pickerOrderMode: null, catalogRefresh: { status: "pending", degraded: true } }; + await reply(2, { ok: true, ...accepted, chosen: ["stale/receipt-choice"], pickerAvailable: ["stale/candidate"] }); + expect(receipts).toEqual([accepted]); expect(order()).toEqual(changedDraft); + expect(host.textContent).toContain("Order saved. Reload current settings before editing again."); + expect(button("Save draft").disabled).toBe(true); expect(button("Move p/a down").disabled).toBe(true); + expect(busy.at(-1)).toBe(false); expect(requests).toHaveLength(3); + await click("Reload and discard draft"); + await reply(3, { ...initial(), pickerOrder: changedDraft }); + expect(button("Move p/a down").disabled).toBe(false); +}); + +const stages = ["initial GET", "preflight GET", "preflight body", "PUT", "receipt body"] as const; +type Stage = typeof stages[number]; +async function pauseAt(stage: Stage): Promise<() => Promise> { + await render(); + if (stage === "initial GET") return () => reply(0, initial()); + await reply(0, initial()); await click("Move p/a down"); await click("Save draft"); + if (stage === "preflight GET") return () => reply(1, initial()); + if (stage !== "preflight body") await reply(1, initial()); + const accepted = { ok: true, pickerOrder: changedDraft, pickerOrderMode: null, catalogRefresh: { status: "pending" } }; + if (stage === "PUT") return () => reply(2, accepted); + const body = deferred(); let reads = 0; + const response = new Response(); + Object.defineProperty(response, "text", { value: () => { reads++; return body.promise; } }); + await act(async () => { requests[stage === "preflight body" ? 1 : 2]!.resolve(response); }); + expect(reads).toBe(1); // The deferred body is actually reached before changing owner/identity. + return async () => { await act(async () => { body.resolve(JSON.stringify(stage === "preflight body" ? initial() : accepted)); }); }; +} + +for (const stage of stages) { + test(`late ${stage} after unmount cannot write, publish a receipt or reset busy`, async () => { + const settle = await pauseAt(stage), count = requests.length; + await act(async () => { root!.unmount(); root = null; }); + const settledBusy = [...busy]; + expect(requests[count - 1]!.signal?.aborted).toBe(true); + await settle(); + expect(requests).toHaveLength(count); expect(receipts).toEqual([]); + expect(busy).toEqual(settledBusy); expect(host.textContent).toBe(""); + }); + test(`late ${stage} from API A→B→A cannot affect the new A flight`, async () => { + const settle = await pauseAt(stage); + await render("/b"); await render("/a"); + const count = requests.length, current = count - 1, settledBusy = [...busy]; + expect(requests[current]?.url).toBe("/a/api/subagent-models"); expect(busy.at(-1)).toBe(true); + expect(requests[current - 1]!.signal?.aborted).toBe(true); + await settle(); + expect(requests).toHaveLength(count); expect(receipts).toEqual([]); expect(order()).toEqual([]); + expect(busy).toEqual(settledBusy); // Old finally must not clear the successor's busy state. + await reply(current, { ...initial(), pickerOrder: ["p/c", "p/a", "p/b"] }); + expect(order()).toEqual(["p/f", "p/c", "p/a", "p/b"]); + }); + test(`identity drift during ${stage} suppresses stale snapshot, PUT and receipt publication`, async () => { + const settle = await pauseAt(stage), count = requests.length; + await render("/a", ids.map(row => row.id === "a" ? { ...row, id: "raw/a" } : row)); + await settle(); + expect(requests).toHaveLength(count); expect(receipts).toEqual([]); expect(busy.at(-1)).toBe(false); + expect(order()).toEqual(stage === "initial GET" ? [] : changedDraft); + expect(button("Save draft").disabled).toBe(true); + if (stage !== "initial GET") expect(host.textContent).toContain("Picker settings changed."); + // Reload, not the stale operation, is allowed to accept current identities. + await click("Reload and discard draft"); await reply(count, initial()); + expect(button("Move p/a down").disabled).toBe(false); + }); +} + + +for (const chosen of [[""], [" "]]) test(`blank chosen ${JSON.stringify(chosen)} keeps routed editing available`, async () => { + await render(); await reply(0, { ...initial(), chosen }); + expect(order()).toEqual(["p/a", "p/b", "p/c", "p/f"]); + expect(host.querySelector('[role="alert"]')).toBeNull(); + expect(button("Move p/f up").disabled).toBe(false); + await click("Move p/a down"); expect(button("Save draft").disabled).toBe(false); +}); + +for (const availability of ["absent", "throws"] as const) + test(`LAN drag with randomUUID ${availability}: same-editor works; cross-editor and stale tokens fail`, async () => { + Object.defineProperty(globalThis, "crypto", { configurable: true, value: availability === "absent" ? {} + : { randomUUID: () => { throw new Error("insecure context"); } } }); + const { createRoot } = await import("react-dom/client"); + await act(async () => { + root = createRoot(host); + root.render({["left", "right"].map(name =>
+ +
)}); + }); + await reply(requests.findIndex(r => r.url === "/left/api/subagent-models"), initial()); + await reply(requests.findIndex(r => r.url === "/right/api/subagent-models"), initial()); + const left = host.querySelector('[data-editor="left"]')!; + const right = host.querySelector('[data-editor="right"]')!; + const original = ["p/f", "p/a", "p/b", "p/c"], type = "application/x-ocx-picker-order"; + const leftDrag = transfer(), rightDrag = transfer(); + await dragEvent(button("Drag p/a", left), "dragstart", leftDrag); + await dragEvent(button("Drag p/a", right), "dragstart", rightDrag); + expect(leftDrag.getData(type)).not.toBe(""); + expect(leftDrag.getData(type)).not.toBe(rightDrag.getData(type)); + // Both editors have active local drags: rejection must compare identities, not just presence. + await dragEvent(row("p/c", right), "drop", leftDrag); expect(order(right)).toEqual(original); + await dragEvent(row("p/c", left), "drop", leftDrag); expect(order(left)).toEqual(changedDraft); + const fresh = transfer(); await dragEvent(button("Drag p/b", left), "dragstart", fresh); + expect(fresh.getData(type)).not.toBe(leftDrag.getData(type)); + await dragEvent(row("p/c", left), "drop", leftDrag); expect(order(left)).toEqual(changedDraft); + await dragEvent(row("p/c", left), "drop", fresh); expect(order(left)).toEqual(changedDraft); + const ended = transfer(); await dragEvent(button("Drag p/b", left), "dragstart", ended); + await dragEvent(row("p/b", left), "dragend", ended); + await dragEvent(row("p/c", left), "drop", ended); expect(order(left)).toEqual(changedDraft); + const retry = transfer(); await dragEvent(button("Drag p/a", right), "dragstart", retry); + await dragEvent(row("p/c", right), "drop", retry); expect(order(right)).toEqual(changedDraft); + expect(requests.map(r => r.method)).toEqual(["GET", "GET"]); expect(receipts).toEqual([]); + }); + + +test("fresh legacy featured settings cannot unlock a row missing from the model identity catalog", async () => { + const settings = { pickerAvailable: ["p/team-model", "p/a"], chosen: ["p/team/model"], pickerOrder: [], pickerOrderMode: null }; + const a = { provider: "p", id: "a", namespaced: "p/a" }; + await render("/a", [a]); await reply(0, settings); + expect(order()).toEqual([]); expect(button("Save draft").disabled).toBe(true); + expect(host.textContent).toContain("Reload the Models page to refresh its catalog"); + await click("Reload and discard draft"); await reply(1, settings); + expect(order()).toEqual([]); // Settings-only reload cannot repair a missing model catalog. + await render("/a", [a, { provider: "p", id: "team/model", namespaced: "p/team-model" }]); + await click("Reload and discard draft"); await reply(2, settings); + expect(order()).toEqual(["p/team-model", "p/a"]); + expect(button("Drag p/team-model").disabled).toBe(true); + expect(requests.map(r => r.method)).toEqual(["GET", "GET", "GET"]); +}); + +test("duplicate featured choices use last occurrence and padded roster strings do not lock rows", async () => { + await render(); await reply(0, { ...initial(), chosen: ["p/a", "p/b", "p/a", " p/c "] }); + expect(order()).toEqual(["p/b", "p/a", "p/c", "p/f"]); + expect(button("Drag p/b").disabled).toBe(true); expect(button("Drag p/a").disabled).toBe(true); + expect(button("Drag p/c").disabled).toBe(false); +}); + +test("Models pins cache-inferred Custom across late parent GET publication, then resets on API change", async () => { + const modelRows = ids.map(row => ({ ...row, disabled: false })); + const catalog = { models: modelRows, providers: [{ name: "p" }], selectedModels: {}, disabled: [], + contextCaps: {}, contextCapValue: 350_000 }; + const custom = { ...initial(), pickerOrder: ["p/c", "p/a", "p/f", "p/b"] }; + for (const base of ["/a", "/b"]) { + win.sessionStorage.setItem(`ocx.models.catalog.v1:${base}`, JSON.stringify(catalog)); + win.sessionStorage.setItem(`ocx.models.catalog.v1:${base}:picker-order`, JSON.stringify(base === "/a" ? custom + : { ...initial(), pickerOrder: [] })); + } + const deferredFetch = globalThis.fetch; + Object.defineProperty(globalThis, "fetch", { configurable: true, value: (input: RequestInfo | URL, init?: RequestInit) => { + const path = String(input); + if (path.endsWith("/api/subagent-models")) return deferredFetch(input, init); + const payload = path.endsWith("/api/models") ? modelRows + : path.endsWith("/api/providers") ? catalog.providers + : path.endsWith("/api/provider-context-caps") ? { caps: {} } + : path.endsWith("/api/selected-models") ? { selected: {} } + : path.endsWith("/api/aliases") ? { providers: {}, models: {}, defaults: { global: false, providers: {} } } + : undefined; + return Promise.resolve(payload === undefined ? new Response(null, { status: 404 }) : Response.json(payload)); + } }); + const { createRoot } = await import("react-dom/client"); + await act(async () => { root = createRoot(host); root.render(); }); + // Parent resource and editor have separate initial reads; resolve both without relying on effect order. + const initialReads = requests.map((request, index) => ({ request, index })); + expect(initialReads).toHaveLength(2); + for (const { index } of initialReads) await reply(index, custom); + expect(order()).toEqual(["p/f", "p/c", "p/a", "p/b"]); + await click("Move p/a down"); const editor = host.querySelector(".picker-order-editor"); + expect(order()).toEqual(["p/f", "p/c", "p/b", "p/a"]); expect(button("Save draft").disabled).toBe(false); + // Integration seam: publish the same parent resource state a late GET would install. + const late = deferred(); + const publication = late.promise.then(value => setClientResourceData("ocx.models.catalog.v1:/a:picker-order", value)); + await act(async () => { late.resolve({ ...initial(), pickerOrderMode: "provider" }); await publication; }); + expect(host.querySelector(".picker-order-editor")).toBe(editor); + expect(order()).toEqual(["p/f", "p/c", "p/b", "p/a"]); expect(button("Save draft").disabled).toBe(false); + expect(requests.every(r => r.method === "GET")).toBe(true); + await act(async () => { root!.render(); }); + expect(host.querySelector(".picker-order-editor")).toBeNull(); +}); diff --git a/gui/tests/model-picker-order.test.ts b/gui/tests/model-picker-order.test.ts index 29c72c0b16..50f79d0b15 100644 --- a/gui/tests/model-picker-order.test.ts +++ b/gui/tests/model-picker-order.test.ts @@ -1,7 +1,7 @@ import { expect, test } from "bun:test"; import { summarizeUsage } from "../../src/usage/summary"; import type { PersistedUsageEntry } from "../../src/usage/log"; -import { isModelPickerUsage, isPickerOrderSaved, isPickerOrderSettings, modelPickerOrder, modelPickerOrderMode } from "../src/model-picker-order"; +import { pickerIdentityCoverage, customPickerRows, normalizePickerIds, pickerSnapshotSignature, movePickerBefore, stepPickerOrder, isModelPickerUsage, isPickerOrderSaved, isPickerOrderSettings, modelPickerOrder, modelPickerOrderMode } from "../src/model-picker-order"; const models = ["zeta/beta", "alpha/zeta", "alpha/alpha"]; @@ -77,3 +77,110 @@ test("real mixed-resolved usage summary never credits an entire legacy bucket to expect(modelPickerOrder("most-used", ["p/c", "p/b", "p/a"], summary.models)) .toEqual(["p/a", "p/b", "p/c"]); }); + + +test("Custom normalizes exact canonical names before provider/raw aliases, without native guesses", () => { + const identities = [ + { provider: "p", id: "team/model", namespaced: "p/team-model" }, + { provider: "p", id: "collision", namespaced: "p/a" }, + { provider: "p", id: "collision", namespaced: "p/b" }, + ]; + expect(normalizePickerIds(["p/team/model", "p/collision", "native", "p/team-model"], + ["p/team-model", "p/a", "p/b"], identities)).toEqual(["p/team-model"]); + expect(normalizePickerIds(["p/team/model"], ["p/team/model", "p/team-model"], identities)).toEqual(["p/team/model"]); +}); + +test("featured rank wins, survivors retain saved order, newcomers follow GET candidate order", () => { + expect(customPickerRows({ pickerAvailable: ["p/new", "p/b", "p/a", "p/top", "p/b"], + chosen: ["native", "p/top", "p/a", "missing/model"], pickerOrder: ["gone/model", "p/b", "p/a"], pickerOrderMode: null, + }, ["new", "b", "a", "top"].map(id => ({ provider: "p", id, namespaced: `p/${id}` })))).toEqual({ fixed: ["p/top", "p/a"], order: ["p/top", "p/a", "p/b", "p/new"] }); + expect(customPickerRows({ pickerAvailable: [], chosen: [], pickerOrder: [], pickerOrderMode: null }, [])) + .toEqual({ fixed: [], order: [] }); +}); + +test("unknown chosen cannot edit; malformed supplied chosen rejects; native saved ids remain untouched", () => { + const settings = { pickerAvailable: ["p/a"], pickerOrder: ["native", "p/a"], pickerOrderMode: null }; + expect(isPickerOrderSettings(settings)).toBe(true); + expect(customPickerRows(settings, [])).toBeNull(); + expect(customPickerRows({ ...settings, chosen: [] }, [])).toBeNull(); + expect(settings.pickerOrder).toEqual(["native", "p/a"]); + expect(customPickerRows({ ...settings, pickerOrder: [] }, [])).toBeNull(); + for (const chosen of [null, undefined, "p/a", [2]]) expect(isPickerOrderSettings({ ...settings, chosen })).toBe(false); + expect(isPickerOrderSettings({ ...settings, chosen: [] })).toBe(true); +}); + +test("snapshot binds base, activation, candidate sequence, chosen, saved order and provenance", () => { + const settings = { pickerAvailable: ["p/b", "p/a"], chosen: [], pickerOrder: ["p/a"], pickerOrderMode: null }; + const expected = '["/a",7,["p/b","p/a"],[],["p/a"],null]'; + expect(pickerSnapshotSignature("/a", 7, settings)).toBe(expected); + expect(pickerSnapshotSignature("/b", 7, settings)).not.toBe(expected); + expect(pickerSnapshotSignature("/a", 9, settings)).not.toBe(expected); // A → B → A + for (const changed of [ + { ...settings, pickerAvailable: ["p/a", "p/b"] }, { ...settings, chosen: ["p/a"] }, + { ...settings, pickerOrder: [] }, { ...settings, pickerOrderMode: "provider" as const }, + { pickerAvailable: settings.pickerAvailable, pickerOrder: settings.pickerOrder, pickerOrderMode: null }, + ]) expect(pickerSnapshotSignature("/a", 7, changed)).not.toBe(expected); +}); + +test("drop-before re-finds target after removal, while keyboard Down swaps adjacent movable rows", () => { + const order = ["p/featured", "p/a", "p/b", "p/c"], fixed = ["p/featured"]; + expect(movePickerBefore(order, "p/a", "p/c", fixed)).toEqual(["p/featured", "p/b", "p/a", "p/c"]); + expect(movePickerBefore(order, "p/c", "p/a", fixed)).toEqual(["p/featured", "p/c", "p/a", "p/b"]); + expect(movePickerBefore(order, "p/a", "p/b", fixed)).toEqual(order); + expect(stepPickerOrder(order, "p/a", 1, fixed)).toEqual(["p/featured", "p/b", "p/a", "p/c"]); + expect(stepPickerOrder(order, "p/c", -1, fixed)).toEqual(["p/featured", "p/a", "p/c", "p/b"]); + for (const [source, target] of [["outside", "p/a"], ["p/a", "outside"], ["p/a", "p/a"], ["p/featured", "p/b"], ["p/b", "p/featured"]]) + expect(movePickerBefore(order, source!, target!, fixed)).toEqual(order); + expect(stepPickerOrder(order, "p/a", -1, fixed)).toEqual(order); + expect(stepPickerOrder(order, "p/c", 1, fixed)).toEqual(order); + expect(order).toEqual(["p/featured", "p/a", "p/b", "p/c"]); +}); + + +test("blank roster strings retain GET compatibility and preset provenance without becoming featured rows", () => { + for (const blank of ["", " "]) { + const settings = { pickerAvailable: models, chosen: [blank], pickerOrder: ["alpha/alpha", "alpha/zeta", "zeta/beta"], + pickerOrderMode: "provider" as const }; + expect(isPickerOrderSettings(settings)).toBe(true); + expect(normalizePickerIds(settings.chosen, models, [])).toEqual([]); + const identities = [{ provider: "alpha", id: "alpha", namespaced: "alpha/alpha" }, + { provider: "alpha", id: "zeta", namespaced: "alpha/zeta" }, { provider: "zeta", id: "beta", namespaced: "zeta/beta" }]; + expect(customPickerRows(settings, identities)).toEqual({ fixed: [], order: ["alpha/alpha", "alpha/zeta", "zeta/beta"] }); + expect(modelPickerOrderMode(models, settings.pickerOrder, settings.pickerOrderMode)).toBe("provider"); + expect(modelPickerOrder("alphabetical", settings.pickerAvailable)).toEqual(["alpha/alpha", "zeta/beta", "alpha/zeta"]); + expect(settings.chosen).toEqual([blank]); // Normalization must not rewrite the saved roster. + expect(isPickerOrderSettings({ ...settings, pickerOrder: [""] })).toBe(false); + expect(isPickerOrderSettings({ ...settings, pickerAvailable: [" "] })).toBe(false); + } + expect(normalizePickerIds(["", " ", "alpha/zeta"], models, [])).toEqual(["alpha/zeta"]); +}); + + +test("incomplete or ambiguous catalog identities block projection, even with canonical candidates", () => { + const settings = { pickerAvailable: ["p/team-model", "p/a"], chosen: ["p/team/model"], pickerOrder: [], pickerOrderMode: null }; + const team = { provider: "p", id: "team/model", namespaced: "p/team-model" }; + const a = { provider: "p", id: "a", namespaced: "p/a" }; + for (const identities of [[], [a], [team], [team, a, { ...team, namespaced: "p/a" }], + [team, a, { ...team, id: "team-model" }]]) { + expect(pickerIdentityCoverage(settings.pickerAvailable, identities)).toBe(false); + expect(customPickerRows(settings, identities)).toBeNull(); + } + expect(pickerIdentityCoverage(settings.pickerAvailable, [team, a, { ...team }])).toBe(true); + expect(customPickerRows(settings, [team, a])).toEqual({ fixed: ["p/team-model"], order: ["p/team-model", "p/a"] }); +}); + +test("featured ranks use last duplicate, exact canonical precedence, and untrimmed roster strings", () => { + const identities = [{ provider: "p", id: "team/model", namespaced: "p/team-model" }, + { provider: "p", id: "a", namespaced: "p/a" }, { provider: "p", id: "b", namespaced: "p/b" }]; + const settings = { pickerAvailable: ["p/team-model", "p/a", "p/b"], pickerOrder: [], pickerOrderMode: null }; + expect(customPickerRows({ ...settings, chosen: ["p/a", "p/b", "p/a"] }, identities)) + .toEqual({ fixed: ["p/b", "p/a"], order: ["p/b", "p/a", "p/team-model"] }); + expect(customPickerRows({ ...settings, chosen: ["p/team/model", "p/b", "p/team-model"] }, identities)) + .toEqual({ fixed: ["p/b", "p/team-model"], order: ["p/b", "p/team-model", "p/a"] }); + expect(customPickerRows({ ...settings, chosen: ["p/team-model", "p/b", "p/team/model"] }, identities)) + .toEqual({ fixed: ["p/team-model", "p/b"], order: ["p/team-model", "p/b", "p/a"] }); + const chosen = [" p/a ", "", " "]; + expect(customPickerRows({ ...settings, chosen, pickerOrder: [" p/a "] }, identities)) + .toEqual({ fixed: [], order: ["p/a", "p/team-model", "p/b"] }); + expect(chosen).toEqual([" p/a ", "", " "]); +}); From dd8fb50b5e15ace4204ec7aa4ffb0d02df293c94 Mon Sep 17 00:00:00 2001 From: t Date: Mon, 7 Sep 2026 21:46:20 +0900 Subject: [PATCH 2/3] fix(gui): reconcile picker lifecycle outside synchronous effects [skip ci] Keep cancellation and stale-response fencing while separating render state reconciliation from automatic transport startup. Preserve immediate user actions and cover unmount before automatic request issuance. Co-authored-by: leonclab <26379556+leonclab@users.noreply.github.com> --- gui/src/components/ModelPickerOrderEditor.tsx | 61 ++++++++++++------- gui/tests/model-picker-order-editor.test.tsx | 18 ++++++ 2 files changed, 58 insertions(+), 21 deletions(-) diff --git a/gui/src/components/ModelPickerOrderEditor.tsx b/gui/src/components/ModelPickerOrderEditor.tsx index 3c379f8cce..1ca60c96b1 100644 --- a/gui/src/components/ModelPickerOrderEditor.tsx +++ b/gui/src/components/ModelPickerOrderEditor.tsx @@ -35,34 +35,48 @@ export default function ModelPickerOrderEditor({ apiBase, active, identities, on const [announcement, setAnnouncement] = useState(""); const [dragging, setDragging] = useState(null); const [over, setOver] = useState(null); - const drag = useRef<{ id: string; token: string } | null>(null); - const generation = useRef(0); - const flight = useRef(null); + const lifetime = useRef({ + generation: 0, + flight: null as BoundedFetch | null, + drag: null as { id: string; token: string } | null, + }); + const [activation, setActivation] = useState({ apiBase, active, onBusyChange }); const identitySignature = JSON.stringify(identities.map(({ provider, id, namespaced }) => [provider, id, namespaced])); const latestIdentitySignature = useRef(identitySignature); useLayoutEffect(() => { latestIdentitySignature.current = identitySignature; }, [identitySignature]); const identityChanged = snapshot !== null && snapshot.identities !== identitySignature; const disabled = !active || busy || !snapshot || blocked !== null || identityChanged; const dirty = snapshot !== null && JSON.stringify(draft) !== JSON.stringify(snapshot.order); - const clearDrag = useCallback(() => { drag.current = null; setDragging(null); setOver(null); }, []); + const clearDrag = useCallback(() => { lifetime.current.drag = null; setDragging(null); setOver(null); }, []); - // Layout cleanup fences even A → B → A and unmount before a pending promise resumes. + // Reconcile before committing children, like the existing display-name dialog. + if (activation.apiBase !== apiBase || activation.active !== active || activation.onBusyChange !== onBusyChange) { + setActivation({ apiBase, active, onBusyChange }); + setSnapshot(null); setDraft([]); setBlocked(null); setError(false); setBusy(false); + } + const [dragContext, setDragContext] = useState({ disabled, snapshot, identitySignature }); + if (dragContext.disabled !== disabled || dragContext.snapshot !== snapshot || dragContext.identitySignature !== identitySignature) { + setDragContext({ disabled, snapshot, identitySignature }); + setDragging(null); setOver(null); + } + + // Capture the stable holder, but always abort its CURRENT flight during cleanup. useLayoutEffect(() => { - generation.current++; - setSnapshot(null); setDraft([]); setBlocked(null); setError(false); + const holder = lifetime.current; + holder.generation++; return () => { - generation.current++; - flight.current?.controller.abort(); flight.current?.clear(); flight.current = null; - drag.current = null; onBusyChange(false); + holder.generation++; + holder.flight?.controller.abort(); holder.flight?.clear(); holder.flight = null; + holder.drag = null; onBusyChange(false); }; }, [apiBase, active, onBusyChange]); - useEffect(() => { clearDrag(); }, [disabled, snapshot, identitySignature, clearDrag]); + useLayoutEffect(() => { lifetime.current.drag = null; }, [disabled, snapshot, identitySignature]); const run = async (save: boolean) => { - if (!active || flight.current || (save && (disabled || !dirty))) return; - const owner = generation.current, bounded = createBoundedFetch(15_000); - flight.current = bounded; setBusy(true); onBusyChange(true); setError(false); clearDrag(); - const owns = () => generation.current === owner && flight.current === bounded; + if (!active || lifetime.current.flight || (save && (disabled || !dirty))) return; + const owner = lifetime.current.generation, bounded = createBoundedFetch(15_000); + lifetime.current.flight = bounded; setBusy(true); onBusyChange(true); setError(false); clearDrag(); + const owns = () => lifetime.current.generation === owner && lifetime.current.flight === bounded; const current = () => owns() && !bounded.signal.aborted && latestIdentitySignature.current === identitySignature; try { @@ -102,11 +116,16 @@ export default function ModelPickerOrderEditor({ apiBase, active, identities, on // Current-identity timeouts surface an error; stale identities retain the draft silently. } finally { bounded.clear(); - if (owns()) { flight.current = null; setBusy(false); onBusyChange(false); } + if (owns()) { lifetime.current.flight = null; setBusy(false); onBusyChange(false); } } }; - const enter = useEffectEvent(() => { void run(false); }); - useEffect(() => { if (active) enter(); }, [apiBase, active]); + const enter = useEffectEvent(async () => { + const holder = lifetime.current, owner = holder.generation; + // Automatic startup is cancellable before issuing transport; event actions stay immediate. + await Promise.resolve(); + if (active && holder.generation === owner) void run(false); + }); + useEffect(() => { if (active) void enter(); }, [apiBase, active, onBusyChange]); const move = (id: string, next: string[]) => { if (disabled) return; @@ -125,13 +144,13 @@ export default function ModelPickerOrderEditor({ apiBase, active, identities, on const fixed = snapshot?.fixed.includes(id) === true; return
  • { - if (!drag.current || drag.current.id === id || !movable(drag.current.id) || !movable(id) + if (!lifetime.current.drag || lifetime.current.drag.id === id || !movable(lifetime.current.drag.id) || !movable(id) || !event.dataTransfer.types.includes(DRAG_TYPE)) return; event.preventDefault(); event.dataTransfer.dropEffect = "move"; setOver(id); }} onDragLeave={() => setOver(null)} onDrop={event => { - const source = drag.current; + const source = lifetime.current.drag; if (source && source.id !== id && source.token === event.dataTransfer.getData(DRAG_TYPE) && movable(source.id) && movable(id)) { event.preventDefault(); move(source.id, movePickerBefore(draft, source.id, id, snapshot?.fixed ?? [])); } @@ -141,7 +160,7 @@ export default function ModelPickerOrderEditor({ apiBase, active, identities, on aria-label={t("models.pickerOrder.dragModel", { model: id })} onDragStart={event => { if (!movable(id)) { event.preventDefault(); return; } - const token = newDragToken(); drag.current = { id, token }; setDragging(id); + const token = newDragToken(); lifetime.current.drag = { id, token }; setDragging(id); event.dataTransfer.effectAllowed = "move"; event.dataTransfer.setData(DRAG_TYPE, token); }}>