mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-16 12:15:57 +00:00
fix(ui): harden target update flows in profile editors
- send strict target-only payloads when cliproxy edit only changes target - route profile target updates through shared api client error handling - block concurrent save/target actions in header controls
This commit is contained in:
@@ -14,7 +14,7 @@ import { Label } from '@/components/ui/label';
|
||||
import { Tabs, TabsList, TabsTrigger, TabsContent } from '@/components/ui/tabs';
|
||||
import { useUpdateVariant } from '@/hooks/use-cliproxy';
|
||||
import { CLIPROXY_PROVIDERS, getProviderDisplayName } from '@/lib/provider-config';
|
||||
import type { Variant } from '@/lib/api-client';
|
||||
import type { UpdateVariant, Variant } from '@/lib/api-client';
|
||||
|
||||
const singleProviderSchema = z.object({
|
||||
provider: z.enum(CLIPROXY_PROVIDERS, { message: 'Provider is required' }),
|
||||
@@ -59,6 +59,63 @@ const providerOptions = CLIPROXY_PROVIDERS.map((id) => ({
|
||||
label: getProviderDisplayName(id),
|
||||
}));
|
||||
|
||||
const COMPOSITE_TIERS = ['opus', 'sonnet', 'haiku'] as const;
|
||||
|
||||
function normalizeOptionalValue(value?: string): string | undefined {
|
||||
const trimmed = value?.trim();
|
||||
return trimmed ? trimmed : undefined;
|
||||
}
|
||||
|
||||
function isSingleVariantOnlyTargetChange(variant: Variant, data: SingleProviderFormData): boolean {
|
||||
const currentTarget = variant.target || 'claude';
|
||||
const currentModel = normalizeOptionalValue(variant.model);
|
||||
const currentAccount = normalizeOptionalValue(variant.account);
|
||||
const nextModel = normalizeOptionalValue(data.model);
|
||||
const nextAccount = normalizeOptionalValue(data.account);
|
||||
|
||||
return (
|
||||
data.target !== currentTarget &&
|
||||
data.provider === variant.provider &&
|
||||
nextModel === currentModel &&
|
||||
nextAccount === currentAccount
|
||||
);
|
||||
}
|
||||
|
||||
function normalizeCompositeTier(tier: { provider: string; model: string; account?: string }) {
|
||||
return {
|
||||
provider: tier.provider,
|
||||
model: tier.model.trim(),
|
||||
account: normalizeOptionalValue(tier.account),
|
||||
};
|
||||
}
|
||||
|
||||
function isCompositeVariantOnlyTargetChange(variant: Variant, data: CompositeFormData): boolean {
|
||||
const currentTarget = variant.target || 'claude';
|
||||
const existingTiers = variant.tiers;
|
||||
|
||||
if (!existingTiers || !variant.default_tier) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (data.target === currentTarget) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (data.default_tier !== variant.default_tier) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return COMPOSITE_TIERS.every((tier) => {
|
||||
const current = normalizeCompositeTier(existingTiers[tier]);
|
||||
const next = normalizeCompositeTier(data.tiers[tier]);
|
||||
return (
|
||||
next.provider === current.provider &&
|
||||
next.model === current.model &&
|
||||
next.account === current.account
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
export function CliproxyEditDialog({ variant, open, onOpenChange }: CliproxyEditDialogProps) {
|
||||
const updateMutation = useUpdateVariant();
|
||||
const isComposite = variant?.type === 'composite';
|
||||
@@ -102,10 +159,40 @@ export function CliproxyEditDialog({ variant, open, onOpenChange }: CliproxyEdit
|
||||
|
||||
const onSubmitSingle = async (data: SingleProviderFormData) => {
|
||||
if (!variant) return;
|
||||
// Filter out undefined values - backend interprets undefined as "no change"
|
||||
const payload = Object.fromEntries(
|
||||
Object.entries(data).filter(([, v]) => v !== undefined && v !== '')
|
||||
) as SingleProviderFormData;
|
||||
|
||||
let payload: UpdateVariant = {};
|
||||
|
||||
if (isSingleVariantOnlyTargetChange(variant, data)) {
|
||||
payload = { target: data.target };
|
||||
} else {
|
||||
const currentTarget = variant.target || 'claude';
|
||||
const currentModel = normalizeOptionalValue(variant.model);
|
||||
const currentAccount = normalizeOptionalValue(variant.account);
|
||||
const nextModel = normalizeOptionalValue(data.model);
|
||||
const nextAccount = normalizeOptionalValue(data.account);
|
||||
|
||||
if (data.provider !== variant.provider) {
|
||||
payload.provider = data.provider;
|
||||
}
|
||||
|
||||
if (nextModel !== currentModel) {
|
||||
payload.model = nextModel;
|
||||
}
|
||||
|
||||
if (nextAccount !== currentAccount) {
|
||||
payload.account = nextAccount;
|
||||
}
|
||||
|
||||
if (data.target !== currentTarget) {
|
||||
payload.target = data.target;
|
||||
}
|
||||
}
|
||||
|
||||
if (Object.keys(payload).length === 0) {
|
||||
onOpenChange(false);
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
await updateMutation.mutateAsync({ name: variant.name, data: payload });
|
||||
onOpenChange(false);
|
||||
@@ -116,14 +203,53 @@ export function CliproxyEditDialog({ variant, open, onOpenChange }: CliproxyEdit
|
||||
|
||||
const onSubmitComposite = async (data: CompositeFormData) => {
|
||||
if (!variant) return;
|
||||
|
||||
let payload: UpdateVariant = {};
|
||||
|
||||
if (isCompositeVariantOnlyTargetChange(variant, data)) {
|
||||
payload = { target: data.target };
|
||||
} else {
|
||||
const existingTiers = variant.tiers;
|
||||
const normalizedTiers: NonNullable<UpdateVariant['tiers']> = {
|
||||
opus: normalizeCompositeTier(data.tiers.opus),
|
||||
sonnet: normalizeCompositeTier(data.tiers.sonnet),
|
||||
haiku: normalizeCompositeTier(data.tiers.haiku),
|
||||
};
|
||||
|
||||
const tiersChanged = !existingTiers
|
||||
? true
|
||||
: COMPOSITE_TIERS.some((tier) => {
|
||||
const current = normalizeCompositeTier(existingTiers[tier]);
|
||||
const next = normalizedTiers[tier];
|
||||
return (
|
||||
next.provider !== current.provider ||
|
||||
next.model !== current.model ||
|
||||
next.account !== current.account
|
||||
);
|
||||
});
|
||||
|
||||
if (variant.default_tier !== data.default_tier) {
|
||||
payload.default_tier = data.default_tier;
|
||||
}
|
||||
|
||||
if ((variant.target || 'claude') !== data.target) {
|
||||
payload.target = data.target;
|
||||
}
|
||||
|
||||
if (tiersChanged) {
|
||||
payload.tiers = normalizedTiers;
|
||||
}
|
||||
}
|
||||
|
||||
if (Object.keys(payload).length === 0) {
|
||||
onOpenChange(false);
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
await updateMutation.mutateAsync({
|
||||
name: variant.name,
|
||||
data: {
|
||||
default_tier: data.default_tier,
|
||||
target: data.target,
|
||||
tiers: data.tiers,
|
||||
},
|
||||
data: payload,
|
||||
});
|
||||
onOpenChange(false);
|
||||
} catch (error) {
|
||||
|
||||
@@ -49,6 +49,9 @@ export function HeaderSection({
|
||||
onDelete,
|
||||
onSave,
|
||||
}: HeaderSectionProps) {
|
||||
const isMutating = isSaving || isTargetSaving;
|
||||
const disableHeaderActions = isLoading || isMutating;
|
||||
|
||||
return (
|
||||
<div className="px-6 py-4 border-b bg-background flex items-center justify-between shrink-0">
|
||||
<div>
|
||||
@@ -68,8 +71,15 @@ export function HeaderSection({
|
||||
)}
|
||||
<div className="mt-2 flex items-center gap-2">
|
||||
<span className="text-xs text-muted-foreground">Default target:</span>
|
||||
<Select value={target} onValueChange={(value) => onTargetChange(value as CliTarget)}>
|
||||
<SelectTrigger className="h-7 w-[170px] text-xs" disabled={isTargetSaving}>
|
||||
<Select
|
||||
value={target}
|
||||
onValueChange={(value) => {
|
||||
if (disableHeaderActions) return;
|
||||
onTargetChange(value as CliTarget);
|
||||
}}
|
||||
disabled={disableHeaderActions}
|
||||
>
|
||||
<SelectTrigger className="h-7 w-[170px] text-xs" disabled={disableHeaderActions}>
|
||||
<SelectValue />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
@@ -81,15 +91,15 @@ export function HeaderSection({
|
||||
</div>
|
||||
</div>
|
||||
<div className="flex items-center gap-2">
|
||||
<Button variant="ghost" size="sm" onClick={onRefresh} disabled={isLoading}>
|
||||
<Button variant="ghost" size="sm" onClick={onRefresh} disabled={disableHeaderActions}>
|
||||
<RefreshCw className={`w-4 h-4 ${isLoading ? 'animate-spin' : ''}`} />
|
||||
</Button>
|
||||
{onDelete && (
|
||||
<Button variant="ghost" size="sm" onClick={onDelete}>
|
||||
<Button variant="ghost" size="sm" onClick={onDelete} disabled={isMutating}>
|
||||
<Trash2 className="w-4 h-4 text-destructive" />
|
||||
</Button>
|
||||
)}
|
||||
<Button size="sm" onClick={onSave} disabled={isSaving || !hasChanges || !isRawJsonValid}>
|
||||
<Button size="sm" onClick={onSave} disabled={isMutating || !hasChanges || !isRawJsonValid}>
|
||||
{isSaving ? (
|
||||
<>
|
||||
<Loader2 className="w-4 h-4 mr-1 animate-spin" />
|
||||
|
||||
@@ -15,7 +15,7 @@ import { HeaderSection } from './header-section';
|
||||
import { FriendlyUISection } from './friendly-ui-section';
|
||||
import { RawEditorSection } from './raw-editor-section';
|
||||
import type { ProfileEditorProps, Settings, SettingsResponse } from './types';
|
||||
import type { CliTarget } from '@/lib/api-client';
|
||||
import { api, type CliTarget } from '@/lib/api-client';
|
||||
|
||||
export function ProfileEditor({
|
||||
profileName,
|
||||
@@ -151,39 +151,24 @@ export function ProfileEditor({
|
||||
},
|
||||
});
|
||||
|
||||
const targetMutation = useMutation({
|
||||
const targetMutation = useMutation<CliTarget, Error, CliTarget>({
|
||||
mutationFn: async (target: CliTarget) => {
|
||||
const response = await fetch(`/api/profiles/${profileName}`, {
|
||||
method: 'PUT',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ target }),
|
||||
});
|
||||
|
||||
if (!response.ok) {
|
||||
let errorMessage = 'Failed to update target';
|
||||
try {
|
||||
const payload = (await response.json()) as { error?: string };
|
||||
if (payload.error) {
|
||||
errorMessage = payload.error;
|
||||
}
|
||||
} catch {
|
||||
// Keep fallback error message.
|
||||
}
|
||||
throw new Error(errorMessage);
|
||||
}
|
||||
|
||||
return response.json();
|
||||
await api.profiles.update(profileName, { target });
|
||||
return target;
|
||||
},
|
||||
onSuccess: () => {
|
||||
queryClient.invalidateQueries({ queryKey: ['profiles'] });
|
||||
toast.success('Default target updated');
|
||||
},
|
||||
onError: (error: Error) => {
|
||||
toast.error(error.message);
|
||||
onError: (error: Error, target: CliTarget) => {
|
||||
const targetLabel = target === 'droid' ? 'Factory Droid' : 'Claude Code';
|
||||
const suffix = error.message.trim() ? `: ${error.message}` : '';
|
||||
toast.error(`Failed to update default target to ${targetLabel}${suffix}`);
|
||||
},
|
||||
});
|
||||
|
||||
const resolvedTarget: CliTarget = profileTarget || 'claude';
|
||||
const isHeaderMutationPending = saveMutation.isPending || targetMutation.isPending;
|
||||
|
||||
const handleConflictResolve = async (overwrite: boolean) => {
|
||||
setConflictDialog(false);
|
||||
@@ -209,12 +194,19 @@ export function ProfileEditor({
|
||||
hasChanges={computedHasChanges}
|
||||
isRawJsonValid={computedIsRawJsonValid}
|
||||
onTargetChange={(target) => {
|
||||
if (isHeaderMutationPending) return;
|
||||
if (target === resolvedTarget) return;
|
||||
targetMutation.mutate(target);
|
||||
}}
|
||||
onRefresh={() => refetch()}
|
||||
onRefresh={() => {
|
||||
if (isHeaderMutationPending) return;
|
||||
refetch();
|
||||
}}
|
||||
onDelete={onDelete}
|
||||
onSave={() => saveMutation.mutate()}
|
||||
onSave={() => {
|
||||
if (isHeaderMutationPending) return;
|
||||
saveMutation.mutate();
|
||||
}}
|
||||
/>
|
||||
|
||||
{isLoading ? (
|
||||
|
||||
Reference in New Issue
Block a user