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 <noreply@anthropic.com>
This commit is contained in:
m4
@@ -842,7 +842,6 @@ export function RegistryEditor() {
|
||||
);
|
||||
const [selectedTemplate, setSelectedTemplate] = useState<string>("zhipu-glm");
|
||||
const [selectedProviderIndex, setSelectedProviderIndex] = useState(0);
|
||||
const [flowStep, setFlowStep] = useState<string | null>(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 && (
|
||||
<div className="rounded-md border border-[var(--color-warning)]/30 bg-[var(--color-warning)]/5 px-4 py-3 text-xs">
|
||||
<p className="font-medium">
|
||||
Changed models require re-verification:{" "}
|
||||
Changed models will be saved as disabled:{" "}
|
||||
{reverifyRefs.map(modelRefKey).join(", ")}
|
||||
</p>
|
||||
<p className="mt-1 text-muted-foreground">
|
||||
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.
|
||||
</p>
|
||||
</div>
|
||||
)}
|
||||
@@ -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"}
|
||||
</Button>
|
||||
<Button
|
||||
|
||||
@@ -100,11 +100,28 @@ describe("modelsNeedingReverify", () => {
|
||||
} as unknown as RegistryV4;
|
||||
}
|
||||
|
||||
function dataFor(registry: RegistryV4): GetModelRegistryResponse {
|
||||
return { revision: 1, registry } as unknown as GetModelRegistryResponse;
|
||||
function dataFor(
|
||||
registry: RegistryV4,
|
||||
verificationStatus: "none" | "passed" | "failed" | "stale" = "passed"
|
||||
): GetModelRegistryResponse {
|
||||
const model_status = registry.providers.flatMap((provider) =>
|
||||
provider.models.map((model) => ({
|
||||
model_ref: { provider_id: provider.id, model_key: model.key },
|
||||
state: model.enabled ? "enabled" : "verified",
|
||||
selectable: model.enabled,
|
||||
reason_code: null,
|
||||
verification: { status: verificationStatus },
|
||||
effective_capabilities: {
|
||||
tools: true,
|
||||
vision: false,
|
||||
structured_output: true,
|
||||
},
|
||||
}))
|
||||
);
|
||||
return { revision: 1, registry, model_status } as unknown as GetModelRegistryResponse;
|
||||
}
|
||||
|
||||
it("returns nothing when the enabled model is unchanged", () => {
|
||||
it("returns nothing when the enabled model is unchanged and verified", () => {
|
||||
const registry = registryWith();
|
||||
expect(
|
||||
modelsNeedingReverify(dataFor(registry), structuredClone(registry), new Map())
|
||||
@@ -129,16 +146,26 @@ describe("modelsNeedingReverify", () => {
|
||||
).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("flags a newly enabled model even without config changes", () => {
|
||||
it("flags enabling a model that has no passing verification", () => {
|
||||
const saved = registryWith();
|
||||
saved.providers[0].models[0].enabled = false;
|
||||
const draft = structuredClone(saved);
|
||||
draft.providers[0].models[0].enabled = true;
|
||||
expect(
|
||||
modelsNeedingReverify(dataFor(saved), draft, new Map())
|
||||
modelsNeedingReverify(dataFor(saved, "none"), draft, new Map())
|
||||
).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("allows re-enabling a model that passed its provider test", () => {
|
||||
const saved = registryWith();
|
||||
saved.providers[0].models[0].enabled = false;
|
||||
const draft = structuredClone(saved);
|
||||
draft.providers[0].models[0].enabled = true;
|
||||
expect(
|
||||
modelsNeedingReverify(dataFor(saved, "passed"), draft, new Map())
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it("flags enabled models when their credential is rotated", () => {
|
||||
const saved = registryWith();
|
||||
const draft = structuredClone(saved);
|
||||
|
||||
+19
-12
@@ -216,13 +216,13 @@ export function registryDirty(
|
||||
}
|
||||
|
||||
/**
|
||||
* Enabled draft models whose verification-defining inputs changed relative
|
||||
* to the saved registry (design doc 7.1 item 7 and 9.2 rule 5). The backend
|
||||
* rejects saving an enabled model without a passing verification for the
|
||||
* current `configuration_hash` (adapter, base URL, upstream model id, all
|
||||
* runtime parameters, capabilities and limits), `credential_revision`, and
|
||||
* adapter spec revision, so these models need the wrapped "disable & save →
|
||||
* test → enable & save" re-verify sequence.
|
||||
* Draft-enabled models that cannot be saved as enabled (design doc 7.1 item
|
||||
* 7 and 9.2 rule 5): either their verification-defining inputs changed
|
||||
* relative to the saved registry (adapter, base URL, upstream model id, all
|
||||
* runtime parameters, capabilities and limits — plus credential rotation),
|
||||
* or the backend has no passing verification for the current configuration.
|
||||
* Saving disables them; the user re-verifies with a manual provider test and
|
||||
* re-enables afterwards.
|
||||
*/
|
||||
export function modelsNeedingReverify(
|
||||
data: GetModelRegistryResponse,
|
||||
@@ -246,18 +246,25 @@ export function modelsNeedingReverify(
|
||||
JSON.stringify(provider.runtime);
|
||||
for (const model of provider.models) {
|
||||
if (!model.enabled) continue;
|
||||
const ref = { provider_id: provider.id, model_key: model.key };
|
||||
const savedModel = savedProvider?.models.find(
|
||||
(entry) => entry.key === model.key
|
||||
);
|
||||
if (
|
||||
const configChanged =
|
||||
!savedModel ||
|
||||
!savedModel.enabled ||
|
||||
providerConfigChanged ||
|
||||
credentialRotated ||
|
||||
savedModel.upstream_model_id !== model.upstream_model_id ||
|
||||
JSON.stringify(savedModel.runtime) !== JSON.stringify(model.runtime)
|
||||
) {
|
||||
refs.push({ provider_id: provider.id, model_key: model.key });
|
||||
JSON.stringify(savedModel.runtime) !== JSON.stringify(model.runtime);
|
||||
if (configChanged) {
|
||||
refs.push(ref);
|
||||
continue;
|
||||
}
|
||||
// Config unchanged: enabling is allowed only when the backend already
|
||||
// holds a passing verification for exactly this configuration.
|
||||
const availability = availabilityFor(data, ref);
|
||||
if (availability?.verification.status !== "passed") {
|
||||
refs.push(ref);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user