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/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; + }, + }; +}; 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(); 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', ]) {