From 340136ebc16b5457a9360f61cf620c37d08bbbc0 Mon Sep 17 00:00:00 2001 From: m4 Date: Wed, 22 Jul 2026 10:11:04 +0800 Subject: [PATCH] feat: save changed models as disabled instead of auto-testing Saving no longer runs live provider tests: changed enabled models are saved as disabled (defaults repointed when needed) and the user re-verifies manually with the per-model Test button, then re-enables and saves again. A successful test now refreshes the local availability so re-enabling is a plain save. Co-Authored-By: Claude Opus 4.7 --- src/app/components/RegistryEditor.tsx | 199 ++++++++++---------------- src/lib/registryDraft.test.ts | 37 ++++- src/lib/registryDraft.ts | 31 ++-- 3 files changed, 129 insertions(+), 138 deletions(-) diff --git a/src/app/components/RegistryEditor.tsx b/src/app/components/RegistryEditor.tsx index bd9730e..0ebf907 100644 --- a/src/app/components/RegistryEditor.tsx +++ b/src/app/components/RegistryEditor.tsx @@ -842,7 +842,6 @@ export function RegistryEditor() { ); const [selectedTemplate, setSelectedTemplate] = useState("zhipu-glm"); const [selectedProviderIndex, setSelectedProviderIndex] = useState(0); - const [flowStep, setFlowStep] = useState(null); const load = useCallback(async () => { setLoadError(null); @@ -889,6 +888,25 @@ export function RegistryEditor() { [data, draft, credentialWrites] ); + /** Where the default primary will point after saving, if the edited model + * currently holds that default (null when no repoint is needed). */ + const repointTarget = useMemo((): ModelRef | null => { + if (!data || !draft || reverifyRefs.length === 0) return null; + const primary = draft.defaults.primary; + if (!primary) return null; + const affectedKeys = new Set(reverifyRefs.map(modelRefKey)); + if (!affectedKeys.has(modelRefKey(primary))) return null; + const remaining = structuredClone(draft); + for (const provider of remaining.providers) { + for (const model of provider.models) { + if (affectedKeys.has(`${provider.id}/${model.key}`)) { + model.enabled = false; + } + } + } + return enabledModelRefs(remaining)[0]?.ref ?? null; + }, [data, draft, reverifyRefs]); + const stagedWrites = useCallback((): CredentialWrite[] => { const writes: CredentialWrite[] = []; for (const [credentialId, secret] of credentialWrites) { @@ -939,154 +957,70 @@ export function RegistryEditor() { } }, [data, draft, stagedWrites, load]); - /** Wrapped "disable & save → test → enable & save" re-verify sequence - * (design doc 7.1 item 7): the backend still sees two independent saves. */ - const reverifyAndSave = useCallback( + /** Save with unverifiable models disabled (design doc 9.2 rule 5): the + * backend only stores an enabled model alongside a passing verification, + * so changed models are saved as disabled. The user re-verifies them + * manually with the per-model Test button and re-enables afterwards. */ + const saveDisablingChanged = useCallback( async (affected: ModelRef[]) => { if (!data || !draft) return; setSaving(true); setSaveIssues(null); const affectedKeys = new Set(affected.map(modelRefKey)); - const setAffectedEnabled = (registry: RegistryV4, enabled: boolean) => { - for (const provider of registry.providers) { - for (const model of provider.models) { - if (affectedKeys.has(`${provider.id}/${model.key}`)) { - model.enabled = enabled; - } - } - } - }; const isAffected = (ref: ModelRef | null) => ref !== null && affectedKeys.has(modelRefKey(ref)); try { - setFlowStep("Saving with changed models disabled…"); - const draft1 = structuredClone(draft); - setAffectedEnabled(draft1, false); - const restorePrimary = draft1.defaults.primary; - const restoreAuxiliary = draft1.defaults.auxiliary; - if (isAffected(draft1.defaults.auxiliary)) { - draft1.defaults.auxiliary = null; + const next = structuredClone(draft); + for (const provider of next.providers) { + for (const model of provider.models) { + if (affectedKeys.has(`${provider.id}/${model.key}`)) { + model.enabled = false; + } + } } - if (isAffected(draft1.defaults.primary)) { - const replacement = enabledModelRefs(draft1)[0]?.ref ?? null; + if (isAffected(next.defaults.auxiliary)) { + next.defaults.auxiliary = null; + } + if (isAffected(next.defaults.primary)) { + const replacement = enabledModelRefs(next)[0]?.ref ?? null; if (!replacement) { toast.error( - "Editing the default primary model requires a second enabled model to take over temporarily." + "Editing the default primary model requires a second enabled model to take over." ); return; } - draft1.defaults.primary = replacement; + next.defaults.primary = replacement; } const writes = stagedWrites(); - const save1 = await putRegistry({ + const result = await putRegistry({ expected_revision: data.revision, - registry: draft1, + registry: next, ...(writes.length > 0 ? { credential_writes: writes } : {}), }); - if (!save1.ok) { - if (save1.status === 409) { + if (!result.ok) { + if (result.status === 409) { toast.error( "Registry changed elsewhere — reloaded the latest state." ); await load(); return; } - setSaveIssues(save1.error); - toast.error(save1.error.message); + setSaveIssues(result.error); + toast.error(result.error.message); return; } - const current = save1.body; - setData(current); - setDraft(structuredClone(current.registry)); - - const failures: string[] = []; - for (const ref of affected) { - const key = modelRefKey(ref); - setFlowStep(`Testing ${key}…`); - setTesting((currentTesting) => ({ ...currentTesting, [key]: true })); - try { - const response = await fetch("/api/model-registry/test", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ - expected_registry_revision: current.revision, - model_ref: ref, - }), - }); - if (!response.ok) { - const error = await readApiError(response); - setTestOutcomes((currentOutcomes) => ({ - ...currentOutcomes, - [key]: { ok: false, message: error.message, code: error.code }, - })); - failures.push(`${key}: ${error.message}`); - continue; - } - const result = (await response.json()) as ProviderTestResponse; - if (result.ok) { - setTestOutcomes((currentOutcomes) => ({ - ...currentOutcomes, - [key]: { ok: true, result }, - })); - } else { - setTestOutcomes((currentOutcomes) => ({ - ...currentOutcomes, - [key]: { - ok: false, - message: "The provider rejected the test request.", - code: null, - }, - })); - failures.push(`${key}: the provider rejected the test request.`); - } - } finally { - setTesting((currentTesting) => ({ - ...currentTesting, - [key]: false, - })); - } - } - if (failures.length > 0) { - toast.error( - `Re-verification failed; the model stays disabled. ${failures[0]}` - ); - return; - } - - setFlowStep("Re-enabling verified models…"); - const draft2 = structuredClone(current.registry); - setAffectedEnabled(draft2, true); - draft2.defaults.primary = restorePrimary; - draft2.defaults.auxiliary = restoreAuxiliary; - const save2 = await putRegistry({ - expected_revision: current.revision, - registry: draft2, - }); - if (!save2.ok) { - if (save2.status === 409) { - toast.error( - "Registry changed elsewhere — reloaded the latest state." - ); - await load(); - return; - } - setSaveIssues(save2.error); - toast.error(save2.error.message); - return; - } - setData(save2.body); - setDraft(structuredClone(save2.body.registry)); + setData(result.body); + setDraft(structuredClone(result.body.registry)); setCredentialWrites(new Map()); invalidateAvailableModels(); toast.success( - `Registry saved and re-verified (revision ${save2.body.revision}).` + `Registry saved (revision ${result.body.revision}); changed models are disabled — test and re-enable them when ready.` ); } catch (reason) { toast.error( - reason instanceof Error ? reason.message : "Re-verification failed." + reason instanceof Error ? reason.message : "Failed to save the registry." ); } finally { - setFlowStep(null); setSaving(false); } }, @@ -1124,6 +1058,23 @@ export function RegistryEditor() { ...current, [key]: { ok: true, result }, })); + // The backend wrote a passing verification row; refresh the local + // availability so re-enabling the model no longer counts as + // unverified (modelsNeedingReverify reads model_status). + setData((current) => + current + ? { + ...current, + model_status: current.model_status.map((status) => + status.model_ref.provider_id === + result.model_ref.provider_id && + status.model_ref.model_key === result.model_ref.model_key + ? result.model_status + : status + ), + } + : current + ); toast.success(`${key} passed the provider test.`); invalidateAvailableModels(); } else { @@ -1421,13 +1372,19 @@ export function RegistryEditor() { {reverifyRefs.length > 0 && (

- Changed models require re-verification:{" "} + Changed models will be saved as disabled:{" "} {reverifyRefs.map(modelRefKey).join(", ")}

- Saving will temporarily disable them, save, run provider tests, - then re-enable the ones that pass. They are unavailable to new - runs until re-verification completes. + An enabled model needs a passing provider test at its current + configuration. Saving disables the changed models + {repointTarget + ? ` and points the default primary at ${modelRefKey( + repointTarget + )}` + : ""} + ; use each model's Test button when ready, then re-enable and + save again.

)} @@ -1437,15 +1394,15 @@ export function RegistryEditor() { type="button" onClick={() => reverifyRefs.length > 0 - ? void reverifyAndSave(reverifyRefs) + ? void saveDisablingChanged(reverifyRefs) : void save() } disabled={saving || !dirty} > {saving - ? (flowStep ?? "Saving…") + ? "Saving…" : reverifyRefs.length > 0 - ? "Save & re-verify" + ? "Save (changed models disabled)" : "Save registry"}