From c5d65ebd156489603691deef323f74b74faf8327 Mon Sep 17 00:00:00 2001 From: seakee Date: Mon, 20 Jul 2026 20:55:54 +0800 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=90=9B=20fix(auth-files):=20harden=20?= =?UTF-8?q?OAuth=20configuration=20mutations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep loader callbacks stable so initialization effects do not restart requests after state updates. Serialize multi-channel alias writes and roll back completed and ambiguous failed requests. Add regression coverage for loader identity and rollback behavior. --- .../useAuthFilesOauth.stability.test.tsx | 175 ++++++++++++++++++ .../authFiles/hooks/useAuthFilesOauth.tsx | 54 +++--- .../authFiles/oauthAliasValidation.test.ts | 50 ++++- .../authFiles/oauthAliasValidation.ts | 42 ++++- 4 files changed, 279 insertions(+), 42 deletions(-) create mode 100644 apps/web/src/features/authFiles/hooks/useAuthFilesOauth.stability.test.tsx diff --git a/apps/web/src/features/authFiles/hooks/useAuthFilesOauth.stability.test.tsx b/apps/web/src/features/authFiles/hooks/useAuthFilesOauth.stability.test.tsx new file mode 100644 index 000000000..3e597d60e --- /dev/null +++ b/apps/web/src/features/authFiles/hooks/useAuthFilesOauth.stability.test.tsx @@ -0,0 +1,175 @@ +import { act, createElement, useEffect, useState } from 'react'; +import { create, type ReactTestRenderer } from 'react-test-renderer'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { UseAuthFilesOauthResult } from './useAuthFilesOauth'; + +const { mocks } = vi.hoisted(() => { + const translate = (key: string) => key; + return { + mocks: { + getOauthExcludedModels: vi.fn(), + getOauthModelAlias: vi.fn(), + getModelDefinitions: vi.fn(), + showNotification: vi.fn(), + showConfirmation: vi.fn(), + // Stable like production react-i18next `t` / store actions. + translate, + }, + }; +}); + +vi.mock('react-i18next', () => ({ + useTranslation: () => ({ + t: mocks.translate, + }), + Trans: ({ children }: { children?: unknown }) => children ?? null, +})); + +vi.mock('@/stores', () => ({ + useNotificationStore: () => ({ + showNotification: mocks.showNotification, + showConfirmation: mocks.showConfirmation, + }), +})); + +vi.mock('@/services/api', () => ({ + authFilesApi: { + getOauthExcludedModels: mocks.getOauthExcludedModels, + getOauthModelAlias: mocks.getOauthModelAlias, + getModelDefinitions: mocks.getModelDefinitions, + }, +})); + +import { useAuthFilesOauth } from './useAuthFilesOauth'; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +type HarnessApi = { + getLatest: () => UseAuthFilesOauthResult; + bumpRender: () => void; + unmount: () => void; +}; + +/** + * Mimics AuthFilesPage: call loaders once when they appear in the dependency list. + * Unstable loader identities re-trigger this effect after every successful load. + */ +function AuthFilesInitEffectHarness({ + onResult, + onBumpReady, +}: { + onResult: (result: UseAuthFilesOauthResult) => void; + onBumpReady: (bump: () => void) => void; +}) { + const [renderTick, setRenderTick] = useState(0); + const result = useAuthFilesOauth({ viewMode: 'list', files: [] }); + const { loadExcluded, loadModelAlias } = result; + + useEffect(() => { + onBumpReady(() => setRenderTick((value) => value + 1)); + }, [onBumpReady]); + + useEffect(() => { + void loadExcluded(); + void loadModelAlias(); + }, [loadExcluded, loadModelAlias]); + + // Force a harmless re-render path that must not recreate loader identities. + void renderTick; + onResult(result); + return null; +} + +const mountHarness = (): HarnessApi => { + let latest: UseAuthFilesOauthResult | null = null; + let bumpRender: (() => void) | null = null; + let renderer: ReactTestRenderer | null = null; + + act(() => { + renderer = create( + createElement(AuthFilesInitEffectHarness, { + onResult: (result) => { + latest = result; + }, + onBumpReady: (bump) => { + bumpRender = bump; + }, + }) + ); + }); + + return { + getLatest: () => { + if (!latest) { + throw new Error('useAuthFilesOauth harness did not mount'); + } + return latest; + }, + bumpRender: () => { + if (!bumpRender) { + throw new Error('useAuthFilesOauth harness bump is not ready'); + } + act(() => { + bumpRender?.(); + }); + }, + unmount: () => { + act(() => { + renderer?.unmount(); + }); + renderer = null; + }, + }; +}; + +const flushAsync = async () => { + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); +}; + +describe('useAuthFilesOauth loader reference stability', () => { + let harness: HarnessApi | null = null; + + beforeEach(() => { + mocks.getOauthExcludedModels.mockReset(); + mocks.getOauthModelAlias.mockReset(); + mocks.getModelDefinitions.mockReset(); + mocks.showNotification.mockReset(); + mocks.showConfirmation.mockReset(); + + mocks.getOauthExcludedModels.mockResolvedValue({}); + mocks.getOauthModelAlias.mockResolvedValue({}); + mocks.getModelDefinitions.mockResolvedValue([]); + }); + + afterEach(() => { + harness?.unmount(); + harness = null; + }); + + it('loads each OAuth config endpoint once and keeps loader identities stable across re-renders', async () => { + harness = mountHarness(); + await flushAsync(); + + expect(mocks.getOauthExcludedModels).toHaveBeenCalledTimes(1); + expect(mocks.getOauthModelAlias).toHaveBeenCalledTimes(1); + + const afterFirstLoad = harness.getLatest(); + expect(afterFirstLoad.excludedError).toBe('ready'); + expect(afterFirstLoad.modelAliasError).toBe('ready'); + + const loadExcludedBeforeRerender = afterFirstLoad.loadExcluded; + const loadModelAliasBeforeRerender = afterFirstLoad.loadModelAlias; + + harness.bumpRender(); + await flushAsync(); + + const afterRerender = harness.getLatest(); + expect(afterRerender.loadExcluded).toBe(loadExcludedBeforeRerender); + expect(afterRerender.loadModelAlias).toBe(loadModelAliasBeforeRerender); + expect(mocks.getOauthExcludedModels).toHaveBeenCalledTimes(1); + expect(mocks.getOauthModelAlias).toHaveBeenCalledTimes(1); + }); +}); diff --git a/apps/web/src/features/authFiles/hooks/useAuthFilesOauth.tsx b/apps/web/src/features/authFiles/hooks/useAuthFilesOauth.tsx index 87e37b02c..592db4a98 100644 --- a/apps/web/src/features/authFiles/hooks/useAuthFilesOauth.tsx +++ b/apps/web/src/features/authFiles/hooks/useAuthFilesOauth.tsx @@ -6,6 +6,7 @@ import type { AuthFileItem, OAuthModelAliasEntry } from '@/types'; import type { AuthFileModelItem, OAuthConfigLoadState } from '@/features/authFiles/constants'; import { normalizeProviderKey } from '@/features/authFiles/constants'; import { + applyOAuthAliasWritePlans, createSerialAsyncQueue, findChannelMappings, getHttpStatusCode, @@ -477,31 +478,13 @@ export function useAuthFilesOauth(options: UseAuthFilesOauthOptions): UseAuthFil return; } - // Capture pre-write snapshots so a mid-loop network failure can best-effort roll back. - const previousByChannel = new Map( - planResult.plans.map((plan) => { - const { mappings } = findChannelMappings(latest, plan.channel); - return [plan.channel, mappings] as const; - }) + await applyOAuthAliasWritePlans( + planResult.plans.map((plan) => ({ + ...plan, + previousMappings: findChannelMappings(latest, plan.channel).mappings, + })), + persistChannelMappings ); - const appliedChannels: string[] = []; - - try { - for (const plan of planResult.plans) { - await persistChannelMappings(plan.channel, plan.nextMappings); - appliedChannels.push(plan.channel); - } - } catch (writeErr: unknown) { - for (const channel of appliedChannels) { - const previous = previousByChannel.get(channel) ?? []; - try { - await persistChannelMappings(channel, previous); - } catch { - // Best-effort rollback; surface the original write error below. - } - } - throw writeErr; - } showNotification(t('oauth_model_alias.save_success'), 'success'); }); @@ -534,12 +517,16 @@ export function useAuthFilesOauth(options: UseAuthFilesOauthOptions): UseAuthFil ); if (providersToUpdate.length === 0) return; - for (const [channel, mappings] of providersToUpdate) { - const nextMappings = mappings.filter( - (mapping) => (mapping.alias ?? '').trim().toLowerCase() !== aliasKey - ); - await persistChannelMappings(channel, nextMappings); - } + await applyOAuthAliasWritePlans( + providersToUpdate.map(([channel, mappings]) => ({ + channel, + previousMappings: mappings, + nextMappings: mappings.filter( + (mapping) => (mapping.alias ?? '').trim().toLowerCase() !== aliasKey + ), + })), + persistChannelMappings + ); showNotification(t('oauth_model_alias.delete_success'), 'success'); }); @@ -556,8 +543,11 @@ export function useAuthFilesOauth(options: UseAuthFilesOauthOptions): UseAuthFil modelAliasError, allProviderModels, providerList, - loadExcluded: () => loadExcluded(), - loadModelAlias: () => loadModelAlias(), + // Return the memoized callbacks directly. Wrapping them in new arrow functions + // each render breaks AuthFilesPage init effects that depend on these refs and + // causes an infinite GET loop (loader setState → re-render → new refs → effect). + loadExcluded, + loadModelAlias, deleteExcluded, deleteModelAlias, handleMappingUpdate, diff --git a/apps/web/src/features/authFiles/oauthAliasValidation.test.ts b/apps/web/src/features/authFiles/oauthAliasValidation.test.ts index 98ad6e42b..d0650ebee 100644 --- a/apps/web/src/features/authFiles/oauthAliasValidation.test.ts +++ b/apps/web/src/features/authFiles/oauthAliasValidation.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest'; import { + applyOAuthAliasWritePlans, createSerialAsyncQueue, findChannelMappings, formatOAuthAliasPreview, @@ -9,6 +10,47 @@ import { planOAuthAliasRename, } from './oauthAliasValidation'; +describe('applyOAuthAliasWritePlans', () => { + it('rolls back already-applied channels when a later write fails', async () => { + const stored = new Map([ + ['claude', [{ name: 'claude-upstream', alias: 'shared' }]], + ['codex', [{ name: 'codex-upstream', alias: 'shared' }]], + ]); + const writes: string[] = []; + + let codexWrites = 0; + const persist = async (channel: string, mappings: Array<{ name: string; alias: string }>) => { + writes.push(channel); + stored.set(channel, mappings); + if (channel === 'codex' && codexWrites++ === 0) { + throw new Error('response lost after write'); + } + }; + + await expect( + applyOAuthAliasWritePlans( + [ + { + channel: 'claude', + previousMappings: [{ name: 'claude-upstream', alias: 'shared' }], + nextMappings: [], + }, + { + channel: 'codex', + previousMappings: [{ name: 'codex-upstream', alias: 'shared' }], + nextMappings: [], + }, + ], + persist + ) + ).rejects.toThrow('response lost after write'); + + expect(writes).toEqual(['claude', 'codex', 'codex', 'claude']); + expect(stored.get('claude')).toEqual([{ name: 'claude-upstream', alias: 'shared' }]); + expect(stored.get('codex')).toEqual([{ name: 'codex-upstream', alias: 'shared' }]); + }); +}); + describe('normalizeOAuthAliasEntries', () => { it('accepts multiple aliases for the same upstream name', () => { const result = normalizeOAuthAliasEntries([ @@ -138,9 +180,11 @@ describe('mergeOAuthAliasLink', () => { reason: 'same_as_name', alias: 'same', }); - expect( - mergeOAuthAliasLink([{ name: 'a', alias: 'shared' }], 'b', 'SHARED') - ).toEqual({ kind: 'rejected', reason: 'duplicate_alias', alias: 'SHARED' }); + expect(mergeOAuthAliasLink([{ name: 'a', alias: 'shared' }], 'b', 'SHARED')).toEqual({ + kind: 'rejected', + reason: 'duplicate_alias', + alias: 'SHARED', + }); }); }); diff --git a/apps/web/src/features/authFiles/oauthAliasValidation.ts b/apps/web/src/features/authFiles/oauthAliasValidation.ts index 5a79eda8e..8ba4599ea 100644 --- a/apps/web/src/features/authFiles/oauthAliasValidation.ts +++ b/apps/web/src/features/authFiles/oauthAliasValidation.ts @@ -148,6 +148,39 @@ export type OAuthAliasRenamePlan = { nextMappings: OAuthModelAliasEntry[]; }; +export type OAuthAliasWritePlan = OAuthAliasRenamePlan & { + previousMappings: OAuthModelAliasEntry[]; +}; + +export const applyOAuthAliasWritePlans = async ( + plans: OAuthAliasWritePlan[], + persist: (channel: string, mappings: OAuthModelAliasEntry[]) => Promise +): Promise => { + const appliedPlans: OAuthAliasWritePlan[] = []; + let activePlan: OAuthAliasWritePlan | null = null; + + try { + for (const plan of plans) { + activePlan = plan; + await persist(plan.channel, plan.nextMappings); + appliedPlans.push(plan); + activePlan = null; + } + } catch (writeError: unknown) { + // The failing request may have reached the server before its response was lost, + // so restore that channel too. Rewriting the previous value is safe if it did not apply. + const rollbackPlans = activePlan ? [...appliedPlans, activePlan] : appliedPlans; + for (const plan of [...rollbackPlans].reverse()) { + try { + await persist(plan.channel, plan.previousMappings); + } catch { + // Best-effort rollback. The caller reloads the server state and reports the original error. + } + } + throw writeError; + } +}; + export type OAuthAliasRenamePlanResult = | { ok: true; plans: OAuthAliasRenamePlan[] } | { @@ -242,9 +275,7 @@ export const mergeOAuthAliasLink = ( ) { return { kind: 'unchanged' }; } - if ( - currentMappings.some((mapping) => (mapping.alias ?? '').trim().toLowerCase() === aliasKey) - ) { + if (currentMappings.some((mapping) => (mapping.alias ?? '').trim().toLowerCase() === aliasKey)) { return { kind: 'rejected', reason: 'duplicate_alias', alias: aliasTrim }; } @@ -254,10 +285,7 @@ export const mergeOAuthAliasLink = ( }; }; -export const formatOAuthAliasPreview = ( - mappings: OAuthModelAliasEntry[], - maxItems = 3 -): string => { +export const formatOAuthAliasPreview = (mappings: OAuthModelAliasEntry[], maxItems = 3): string => { if (!mappings.length) return ''; const previews = mappings.slice(0, maxItems).map((entry) => { const name = String(entry.name ?? '').trim(); From 5bd0618be133018b72e19e59b863b5165b6ff363 Mon Sep 17 00:00:00 2001 From: seakee Date: Mon, 20 Jul 2026 20:58:12 +0800 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=90=9B=20fix(ai-providers):=20seriali?= =?UTF-8?q?ze=20provider=20config=20mutations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use a synchronous shared lock for provider toggles, priority changes, and health-check updates. Prevent overlapping optimistic writes from racing and overwriting newer provider configuration. Cover lock acquisition and release behavior with a focused unit test. --- .../features/aiProviders/AiProvidersPage.tsx | 100 ++++++++---------- .../model/configMutationLock.test.ts | 17 +++ .../aiProviders/model/configMutationLock.ts | 21 ++++ 3 files changed, 81 insertions(+), 57 deletions(-) create mode 100644 apps/web/src/features/aiProviders/model/configMutationLock.test.ts create mode 100644 apps/web/src/features/aiProviders/model/configMutationLock.ts diff --git a/apps/web/src/features/aiProviders/AiProvidersPage.tsx b/apps/web/src/features/aiProviders/AiProvidersPage.tsx index dbb9ef789..736ba1c6c 100644 --- a/apps/web/src/features/aiProviders/AiProvidersPage.tsx +++ b/apps/web/src/features/aiProviders/AiProvidersPage.tsx @@ -39,6 +39,7 @@ import type { OpenAIProviderConfig, ProviderKeyConfig, } from '@/types'; +import { createConfigMutationLock } from './model/configMutationLock'; import styles from './AiProvidersPage.module.scss'; const PROVIDER_TABLE_DEFAULT_PAGE_SIZE = 10; @@ -87,8 +88,16 @@ export function AiProvidersPage() { ); const [configSwitchingKey, setConfigSwitchingKey] = useState(null); - /** Synchronous lock so double-clicks in the same tick cannot start two enable/disable writes. */ - const configSwitchingLockRef = useRef(false); + const configMutationLockRef = useRef(createConfigMutationLock()); + const beginConfigMutation = useCallback((switchingKey: string) => { + if (!configMutationLockRef.current.tryAcquire()) return false; + setConfigSwitchingKey(switchingKey); + return true; + }, []); + const finishConfigMutation = useCallback(() => { + configMutationLockRef.current.release(); + setConfigSwitchingKey(null); + }, []); // 表格筛选 / 排序 / 详情状态 const [kindFilter, setKindFilter] = useState('all'); @@ -321,7 +330,7 @@ export function AiProvidersPage() { actions: Map ) => { if (actions.size === 0) return; - if (configSwitchingLockRef.current || configSwitchingKey) return; + if (configMutationLockRef.current.isLocked()) return; const rowByKey = new Map(rows.map((row) => [row.key, row])); const previous = { @@ -431,8 +440,7 @@ export function AiProvidersPage() { return; } - configSwitchingLockRef.current = true; - setConfigSwitchingKey('health-check'); + if (!beginConfigMutation('health-check')) return; const applyLocalState = ( gemini: GeminiKeyConfig[], @@ -556,9 +564,8 @@ export function AiProvidersPage() { showNotification(`${t('notification.update_failed')}: ${message}`, 'error'); throw err; } finally { - configSwitchingLockRef.current = false; - setConfigSwitchingKey(null); - } + finishConfigMutation(); + } }; const setHealthCheckProviderEnabled = async (providerKey: string, enabled: boolean) => { @@ -571,16 +578,13 @@ export function AiProvidersPage() { index: number, enabled: boolean ) => { - if (configSwitchingLockRef.current || configSwitchingKey) return; - if (provider === 'gemini' || provider === 'interactions') { const source = provider === 'gemini' ? geminiKeys : interactionsKeys; const current = source[index]; if (!current) return; const switchingKey = `${provider}:${current.apiKey}`; - configSwitchingLockRef.current = true; - setConfigSwitchingKey(switchingKey); + if (!beginConfigMutation(switchingKey)) return; const previousList = source; const nextExcluded = enabled @@ -623,8 +627,7 @@ export function AiProvidersPage() { } showNotification(`${t('notification.update_failed')}: ${message}`, 'error'); } finally { - configSwitchingLockRef.current = false; - setConfigSwitchingKey(null); + finishConfigMutation(); } return; } @@ -641,8 +644,7 @@ export function AiProvidersPage() { if (!current) return; const switchingKey = `${provider}:${current.apiKey}`; - configSwitchingLockRef.current = true; - setConfigSwitchingKey(switchingKey); + if (!beginConfigMutation(switchingKey)) return; const previousList = source; const nextExcluded = enabled @@ -705,19 +707,16 @@ export function AiProvidersPage() { } showNotification(`${t('notification.update_failed')}: ${message}`, 'error'); } finally { - configSwitchingLockRef.current = false; - setConfigSwitchingKey(null); - } + finishConfigMutation(); + } }; const setOpenAIProviderEnabled = async (index: number, enabled: boolean) => { - if (configSwitchingLockRef.current || configSwitchingKey) return; const current = openaiProviders[index]; if (!current) return; const switchingKey = `openai:${current.name}:${index}`; - configSwitchingLockRef.current = true; - setConfigSwitchingKey(switchingKey); + if (!beginConfigMutation(switchingKey)) return; const previousList = openaiProviders; const nextItem: OpenAIProviderConfig = { ...current, disabled: !enabled }; @@ -741,9 +740,8 @@ export function AiProvidersPage() { clearCache('openai-compatibility'); showNotification(`${t('notification.update_failed')}: ${message}`, 'error'); } finally { - configSwitchingLockRef.current = false; - setConfigSwitchingKey(null); - } + finishConfigMutation(); + } }; const setProviderWebsocketsEnabled = async ( @@ -756,10 +754,8 @@ export function AiProvidersPage() { const current = source[index]; if (!current) return; - if (configSwitchingLockRef.current || configSwitchingKey) return; const switchingKey = `${provider}:${current.apiKey}:websockets`; - configSwitchingLockRef.current = true; - setConfigSwitchingKey(switchingKey); + if (!beginConfigMutation(switchingKey)) return; const previousList = source; const nextItem: ProviderKeyConfig = { ...current, websockets: enabled }; @@ -810,9 +806,8 @@ export function AiProvidersPage() { } showNotification(`${t('notification.update_failed')}: ${message}`, 'error'); } finally { - configSwitchingLockRef.current = false; - setConfigSwitchingKey(null); - } + finishConfigMutation(); + } }; const setProviderCloakEnabled = async ( @@ -824,10 +819,8 @@ export function AiProvidersPage() { const current = source[index]; if (!current) return; - if (configSwitchingLockRef.current || configSwitchingKey) return; const switchingKey = `${provider}:${current.apiKey}:cloak`; - configSwitchingLockRef.current = true; - setConfigSwitchingKey(switchingKey); + if (!beginConfigMutation(switchingKey)) return; const previousList = source; const nextItem: ProviderKeyConfig = enabled @@ -871,9 +864,8 @@ export function AiProvidersPage() { } showNotification(`${t('notification.update_failed')}: ${message}`, 'error'); } finally { - configSwitchingLockRef.current = false; - setConfigSwitchingKey(null); - } + finishConfigMutation(); + } }; const setProviderDisableCoolingEnabled = async ( @@ -887,7 +879,7 @@ export function AiProvidersPage() { if (!current) return; const switchingKey = `${provider}:${current.apiKey}:disable-cooling`; - setConfigSwitchingKey(switchingKey); + if (!beginConfigMutation(switchingKey)) return; const previousList = source; const nextItem: GeminiKeyConfig = { ...current, disableCooling: enabled }; @@ -931,8 +923,7 @@ export function AiProvidersPage() { } showNotification(`${t('notification.update_failed')}: ${message}`, 'error'); } finally { - configSwitchingLockRef.current = false; - setConfigSwitchingKey(null); + finishConfigMutation(); } return; } @@ -942,7 +933,7 @@ export function AiProvidersPage() { if (!current) return; const switchingKey = `${provider}:${current.name}:${index}:disable-cooling`; - setConfigSwitchingKey(switchingKey); + if (!beginConfigMutation(switchingKey)) return; const previousList = openaiProviders; const nextItem: OpenAIProviderConfig = { ...current, disableCooling: enabled }; @@ -963,8 +954,7 @@ export function AiProvidersPage() { clearCache('openai-compatibility'); showNotification(`${t('notification.update_failed')}: ${message}`, 'error'); } finally { - configSwitchingLockRef.current = false; - setConfigSwitchingKey(null); + finishConfigMutation(); } return; } @@ -975,7 +965,7 @@ export function AiProvidersPage() { if (!current) return; const switchingKey = `${provider}:${current.apiKey}:disable-cooling`; - setConfigSwitchingKey(switchingKey); + if (!beginConfigMutation(switchingKey)) return; const previousList = source; const nextItem: ProviderKeyConfig = { ...current, disableCooling: enabled }; @@ -1026,9 +1016,8 @@ export function AiProvidersPage() { } showNotification(`${t('notification.update_failed')}: ${message}`, 'error'); } finally { - configSwitchingLockRef.current = false; - setConfigSwitchingKey(null); - } + finishConfigMutation(); + } }; const setProviderPriority = async (row: ProviderRow, priority: number) => { @@ -1041,7 +1030,7 @@ export function AiProvidersPage() { const current = source[row.originalIndex]; if (!current || current.priority === nextPriority) return; - setConfigSwitchingKey(switchingKey); + if (!beginConfigMutation(switchingKey)) return; const previousList = source; const nextList = previousList.map((item, idx) => idx === row.originalIndex ? { ...item, priority: nextPriority } : item @@ -1085,8 +1074,7 @@ export function AiProvidersPage() { } showNotification(`${t('notification.update_failed')}: ${message}`, 'error'); } finally { - configSwitchingLockRef.current = false; - setConfigSwitchingKey(null); + finishConfigMutation(); } return; } @@ -1095,7 +1083,7 @@ export function AiProvidersPage() { const current = openaiProviders[row.originalIndex]; if (!current || current.priority === nextPriority) return; - setConfigSwitchingKey(switchingKey); + if (!beginConfigMutation(switchingKey)) return; const previousList = openaiProviders; const nextList = previousList.map((item, idx) => idx === row.originalIndex ? { ...item, priority: nextPriority } : item @@ -1119,8 +1107,7 @@ export function AiProvidersPage() { clearCache('openai-compatibility'); showNotification(`${t('notification.update_failed')}: ${message}`, 'error'); } finally { - configSwitchingLockRef.current = false; - setConfigSwitchingKey(null); + finishConfigMutation(); } return; } @@ -1136,7 +1123,7 @@ export function AiProvidersPage() { const current = source[row.originalIndex]; if (!current || current.priority === nextPriority) return; - setConfigSwitchingKey(switchingKey); + if (!beginConfigMutation(switchingKey)) return; const previousList = source; const nextList = previousList.map((item, idx) => idx === row.originalIndex ? { ...item, priority: nextPriority } : item @@ -1199,9 +1186,8 @@ export function AiProvidersPage() { } showNotification(`${t('notification.update_failed')}: ${message}`, 'error'); } finally { - configSwitchingLockRef.current = false; - setConfigSwitchingKey(null); - } + finishConfigMutation(); + } }; // 删除（按 provider 分派，沿用既有 API 契约） diff --git a/apps/web/src/features/aiProviders/model/configMutationLock.test.ts b/apps/web/src/features/aiProviders/model/configMutationLock.test.ts new file mode 100644 index 000000000..d61575722 --- /dev/null +++ b/apps/web/src/features/aiProviders/model/configMutationLock.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, it } from 'vitest'; +import { createConfigMutationLock } from './configMutationLock'; + +describe('createConfigMutationLock', () => { + it('rejects overlapping mutations until the active mutation releases the lock', () => { + const lock = createConfigMutationLock(); + + expect(lock.tryAcquire()).toBe(true); + expect(lock.isLocked()).toBe(true); + expect(lock.tryAcquire()).toBe(false); + + lock.release(); + + expect(lock.isLocked()).toBe(false); + expect(lock.tryAcquire()).toBe(true); + }); +}); diff --git a/apps/web/src/features/aiProviders/model/configMutationLock.ts b/apps/web/src/features/aiProviders/model/configMutationLock.ts new file mode 100644 index 000000000..166c9de2d --- /dev/null +++ b/apps/web/src/features/aiProviders/model/configMutationLock.ts @@ -0,0 +1,21 @@ +export type ConfigMutationLock = { + isLocked: () => boolean; + release: () => void; + tryAcquire: () => boolean; +}; + +export const createConfigMutationLock = (): ConfigMutationLock => { + let locked = false; + + return { + isLocked: () => locked, + release: () => { + locked = false; + }, + tryAcquire: () => { + if (locked) return false; + locked = true; + return true; + }, + }; +}; From d41268fbe9eb2a65bd5020a60f51c942e376763b Mon Sep 17 00:00:00 2001 From: seakee Date: Mon, 20 Jul 2026 20:59:48 +0800 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=90=9B=20fix(ci):=20expand=20PR=20val?= =?UTF-8?q?idation=20coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Classify workflow, native packaging, and Compose changes into the checks they can affect. Validate packaging script syntax and Docker Compose configuration in PR checks. Add classifier regressions so infrastructure changes cannot silently skip validation. --- .github/workflows/pr-check.yml | 8 ++++++++ bin/ci/classify-pr-checks.mjs | 7 +++++-- tests/prCheckClassifier.test.mjs | 26 +++++++++++++++++++++----- 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index 8ed1ed6c8..4fb670744 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -165,6 +165,11 @@ jobs: - name: Test native control scripts run: npx vitest run tests/nativeControlScripts.test.mjs + - name: Validate native packaging script syntax + if: runner.os != 'Windows' + shell: bash + run: bash -n bin/release/package-native.sh + docker-build: name: Docker Build runs-on: ubuntu-latest @@ -185,6 +190,9 @@ jobs: - name: Set up Docker Buildx uses: docker/setup-buildx-action@v3 + - name: Validate Docker Compose configuration + run: docker compose -f docker-compose.manager.yml config + - name: Build Manager Server image uses: docker/build-push-action@v6 with: diff --git a/bin/ci/classify-pr-checks.mjs b/bin/ci/classify-pr-checks.mjs index f4fb4465b..22977b4d7 100644 --- a/bin/ci/classify-pr-checks.mjs +++ b/bin/ci/classify-pr-checks.mjs @@ -23,7 +23,7 @@ const allChecks = (enabled) => Object.fromEntries(CHECK_NAMES.map((checkName) => [checkName, enabled])); const triggersAllChecks = (filePath) => - filePath === '.github/workflows/pr-check.yml' || + startsWithPath(filePath, '.github/workflows') || filePath === 'bin/ci/classify-pr-checks.mjs' || filePath === 'tests/prCheckClassifier.test.mjs'; @@ -37,9 +37,11 @@ const triggersFrontend = (filePath) => filePath === 'package-lock.json' || filePath === 'eslint.config.js' || filePath === 'bin/install-cpamp.sh' || + filePath === 'bin/release/package-native.sh' || filePath === 'bin/release/check-web-demo-isolation.mjs'; -const triggersManagerServer = (filePath) => startsWithPath(filePath, 'apps/manager-server'); +const triggersManagerServer = (filePath) => + startsWithPath(filePath, 'apps/manager-server') || filePath === 'bin/release/package-native.sh'; const triggersNativeControl = (filePath) => startsWithPath(filePath, 'bin/native') || @@ -52,6 +54,7 @@ const triggersDocker = (filePath) => startsWithPath(filePath, 'apps/web') || startsWithPath(filePath, 'apps/manager-server') || filePath === 'Dockerfile.manager-server' || + filePath === 'docker-compose.manager.yml' || filePath === '.dockerignore' || filePath === 'package.json' || filePath === 'package-lock.json'; diff --git a/tests/prCheckClassifier.test.mjs b/tests/prCheckClassifier.test.mjs index 0fcaf4f25..5eacbebe5 100644 --- a/tests/prCheckClassifier.test.mjs +++ b/tests/prCheckClassifier.test.mjs @@ -24,10 +24,8 @@ describe('PR check classifier', () => { }); }); - it('skips application checks for release workflow and ordinary docs changes', () => { - expect(classifyChangedFiles(['.github/workflows/release.yml', 'docs/release.md'])).toEqual( - noChecks - ); + it('skips application checks for ordinary non-site docs changes', () => { + expect(classifyChangedFiles(['docs/release.md'])).toEqual(noChecks); }); it('runs frontend and Docker checks for web changes', () => { @@ -63,6 +61,23 @@ describe('PR check classifier', () => { }); }); + it('runs build coverage for native packaging changes', () => { + expect(classifyChangedFiles(['bin/release/package-native.sh'])).toEqual({ + ...noChecks, + frontend: true, + manager_server: true, + windows_sqlite: true, + native_control: true, + }); + }); + + it('runs Docker validation for Compose changes', () => { + expect(classifyChangedFiles(['docker-compose.manager.yml'])).toEqual({ + ...noChecks, + docker: true, + }); + }); + it('runs Node and Docker checks for root dependency changes', () => { expect(classifyChangedFiles(['package-lock.json'])).toEqual({ ...noChecks, @@ -72,9 +87,10 @@ describe('PR check classifier', () => { }); }); - it('runs all checks when the classifier or PR workflow changes', () => { + it('runs all checks when the classifier or any workflow changes', () => { for (const filePath of [ '.github/workflows/pr-check.yml', + '.github/workflows/release.yml', 'bin/ci/classify-pr-checks.mjs', 'tests/prCheckClassifier.test.mjs', ]) {