From dac45edc415405690efeaec48876cfcdd8fa2109 Mon Sep 17 00:00:00 2001 From: wusumac <736139669@qq.com> Date: Tue, 4 Aug 2026 18:22:40 +0800 Subject: [PATCH] fix(hub,web): serialize notification settings updates --- hub/src/config/settings.ts | 43 ++++++++++++++-- hub/src/tunnel/relayAuth.ts | 50 ++++++++++++------- hub/src/web/routes/notificationCopy.test.ts | 32 ++++++++++++ hub/src/web/routes/notificationCopy.ts | 9 ++-- web/src/lib/locales/en.ts | 1 + web/src/lib/locales/zh-CN.ts | 1 + .../routes/settings/notifications.test.tsx | 10 ++++ web/src/routes/settings/notifications.tsx | 11 +++- 8 files changed, 128 insertions(+), 29 deletions(-) diff --git a/hub/src/config/settings.ts b/hub/src/config/settings.ts index 4b12955d..3284c21c 100644 --- a/hub/src/config/settings.ts +++ b/hub/src/config/settings.ts @@ -4,6 +4,8 @@ import { dirname, join } from 'node:path' import type { NotificationCopyConfig } from '../push/notificationCopy' +const settingsWriteQueues = new Map>() + export interface Settings { machineId?: string machineIdConfirmedByServer?: boolean @@ -60,10 +62,20 @@ export async function readSettingsOrThrow(settingsFile: string): Promise { +async function serializeSettingsWrite(settingsFile: string, operation: () => Promise): Promise { + const previous = settingsWriteQueues.get(settingsFile) ?? Promise.resolve() + const current = previous.catch(() => {}).then(operation) + settingsWriteQueues.set(settingsFile, current) + try { + return await current + } finally { + if (settingsWriteQueues.get(settingsFile) === current) { + settingsWriteQueues.delete(settingsFile) + } + } +} + +async function writeSettingsUnlocked(settingsFile: string, settings: Settings): Promise { const dir = dirname(settingsFile) if (!existsSync(dir)) { await mkdir(dir, { recursive: true, mode: 0o700 }) @@ -73,3 +85,26 @@ export async function writeSettings(settingsFile: string, settings: Settings): P await writeFile(tmpFile, JSON.stringify(settings, null, 2)) await rename(tmpFile, settingsFile) } + +/** + * Write settings to file atomically (temp file + rename). + * Writes are serialized per file so they cannot reuse the same temp path. + */ +export async function writeSettings(settingsFile: string, settings: Settings): Promise { + await serializeSettingsWrite(settingsFile, () => writeSettingsUnlocked(settingsFile, settings)) +} + +/** + * Serialize a complete settings read-modify-write operation per file. + */ +export async function mutateSettings( + settingsFile: string, + mutate: (settings: Settings) => T | Promise +): Promise { + return await serializeSettingsWrite(settingsFile, async () => { + const settings = await readSettingsOrThrow(settingsFile) + const result = await mutate(settings) + await writeSettingsUnlocked(settingsFile, settings) + return result + }) +} diff --git a/hub/src/tunnel/relayAuth.ts b/hub/src/tunnel/relayAuth.ts index 39e04ab8..f6cb3d6f 100644 --- a/hub/src/tunnel/relayAuth.ts +++ b/hub/src/tunnel/relayAuth.ts @@ -9,14 +9,12 @@ * tunnel — there is no shared-key fallback. */ -import { readSettings, writeSettings, type Settings } from '../config/settings' +import { mutateSettings, readSettings } from '../config/settings' type FetchRelayAuth = (input: string | URL | Request, init?: RequestInit) => Promise -async function issueRelayAuthKey( +async function requestRelayAuthKey( apiDomain: string, - settingsFile: string, - settings: Settings | null, fetchRelayAuth: FetchRelayAuth ): Promise { const resp = await fetchRelayAuth(`https://${apiDomain}/issue`, { @@ -41,10 +39,6 @@ async function issueRelayAuthKey( if (typeof data.key !== 'string' || !data.key) { throw new Error(`Relay at ${apiDomain} returned an invalid key response.`) } - // settings === null means the file exists but is unparseable; don't clobber it - if (settings !== null) { - await writeSettings(settingsFile, { ...settings, relayAuthKey: data.key }) - } console.log('[Tunnel] Obtained per-hub relay auth key') return data.key } @@ -64,7 +58,18 @@ export async function resolveRelayAuthKey( return settings.relayAuthKey } - return issueRelayAuthKey(apiDomain, settingsFile, settings, fetchRelayAuth) + const issuedKey = await requestRelayAuthKey(apiDomain, fetchRelayAuth) + // An unreadable file must never be replaced with a partial settings object. + if (settings === null) { + return issuedKey + } + return await mutateSettings(settingsFile, (latestSettings) => { + if (latestSettings.relayAuthKey) { + return latestSettings.relayAuthKey + } + latestSettings.relayAuthKey = issuedKey + return issuedKey + }) } export async function refreshRejectedRelayAuthKey( @@ -80,17 +85,24 @@ export async function refreshRejectedRelayAuthKey( ) } - const settings = await readSettings(settingsFile) - if (settings === null) { - throw new Error(`Cannot refresh relay auth while ${settingsFile} is unreadable.`) - } - if (settings.relayAuthKey && settings.relayAuthKey !== rejectedKey) { - return settings.relayAuthKey + const currentKey = await mutateSettings(settingsFile, (settings) => { + if (settings.relayAuthKey && settings.relayAuthKey !== rejectedKey) { + return settings.relayAuthKey + } + delete settings.relayAuthKey + return null + }) + if (currentKey) { + return currentKey } - const clearedSettings = { ...settings } - delete clearedSettings.relayAuthKey - await writeSettings(settingsFile, clearedSettings) console.warn('[Tunnel] Relay auth key rejected; requesting a replacement') - return issueRelayAuthKey(apiDomain, settingsFile, clearedSettings, fetchRelayAuth) + const issuedKey = await requestRelayAuthKey(apiDomain, fetchRelayAuth) + return await mutateSettings(settingsFile, (settings) => { + if (settings.relayAuthKey) { + return settings.relayAuthKey + } + settings.relayAuthKey = issuedKey + return issuedKey + }) } diff --git a/hub/src/web/routes/notificationCopy.test.ts b/hub/src/web/routes/notificationCopy.test.ts index 8b48f13e..0348dbeb 100644 --- a/hub/src/web/routes/notificationCopy.test.ts +++ b/hub/src/web/routes/notificationCopy.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { Hono } from 'hono' import type { WebAppEnv } from '../middleware/auth' +import { resolveRelayAuthKey } from '../../tunnel/relayAuth' import { createNotificationCopyRoutes } from './notificationCopy' let dir: string @@ -105,6 +106,37 @@ describe('PUT /api/notification-copy', () => { expect(settings.notificationCopy).toEqual({ ready: { title: '', body: '' } }) }) + it('preserves a concurrent relay key update', async () => { + let releaseIssue: () => void = () => {} + let issueStarted: () => void = () => {} + const started = new Promise((resolve) => { + issueStarted = resolve + }) + const release = new Promise((resolve) => { + releaseIssue = resolve + }) + const relayKey = resolveRelayAuthKey('relay.example.com', join(dir, 'settings.json'), async () => { + issueStarted() + await release + return Response.json({ key: 'relay-key' }) + }) + await started + + const app = await createApp('default') + const res = await app.request('/api/notification-copy', { + method: 'PUT', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ ready: { title: 'Hey', body: '{agentName}' } }) + }) + expect(res.status).toBe(200) + releaseIssue() + await relayKey + + const settings = await readSettings() + expect(settings.relayAuthKey).toBe('relay-key') + expect(settings.notificationCopy).toEqual({ ready: { title: 'Hey', body: '{agentName}' } }) + }) + it('rejects title over 500 chars', async () => { const app = await createApp('default') const res = await app.request('/api/notification-copy', { diff --git a/hub/src/web/routes/notificationCopy.ts b/hub/src/web/routes/notificationCopy.ts index f1311356..5ac9a4b3 100644 --- a/hub/src/web/routes/notificationCopy.ts +++ b/hub/src/web/routes/notificationCopy.ts @@ -1,5 +1,5 @@ import { Hono } from 'hono' -import { getSettingsFile, readSettingsOrThrow, writeSettings } from '../../config/settings' +import { getSettingsFile, mutateSettings, readSettingsOrThrow } from '../../config/settings' import { COPY_KEYS, DEFAULT_COPY, @@ -41,8 +41,6 @@ export function createNotificationCopyRoutes(dataDir: string): Hono { return c.json({ error: 'Invalid body', issues: parsed.error.flatten() }, 400) } - // Read-modify-write: preserve every other settings.json key. - const settings = await readSettingsOrThrow(settingsFile) const copy: NotificationCopyConfig = {} for (const key of COPY_KEYS) { const template = parsed.data[key] @@ -50,8 +48,9 @@ export function createNotificationCopyRoutes(dataDir: string): Hono { copy[key as CopyKey] = template } } - settings.notificationCopy = copy - await writeSettings(settingsFile, settings) + await mutateSettings(settingsFile, (settings) => { + settings.notificationCopy = copy + }) return c.json({ copy, defaults: DEFAULT_COPY diff --git a/web/src/lib/locales/en.ts b/web/src/lib/locales/en.ts index e43821c0..d7f79f4a 100644 --- a/web/src/lib/locales/en.ts +++ b/web/src/lib/locales/en.ts @@ -944,6 +944,7 @@ export default { 'settings.notifications.copy.resetDefault': 'Reset to default', 'settings.notifications.copy.save': 'Save copy', 'settings.notifications.copy.saved': 'Copy saved', + 'settings.notifications.copy.saveError': 'Failed to save notification copy', 'settings.notifications.copy.testPushNote': 'The test push button always sends fixed copy.', 'settings.notifications.copy.permissionRequest': 'Permission request', 'settings.notifications.copy.ready': 'Session ready', diff --git a/web/src/lib/locales/zh-CN.ts b/web/src/lib/locales/zh-CN.ts index ac0cb062..753cdb51 100644 --- a/web/src/lib/locales/zh-CN.ts +++ b/web/src/lib/locales/zh-CN.ts @@ -943,6 +943,7 @@ export default { 'settings.notifications.copy.resetDefault': '恢复默认', 'settings.notifications.copy.save': '保存文案', 'settings.notifications.copy.saved': '文案已保存', + 'settings.notifications.copy.saveError': '保存推送文案失败', 'settings.notifications.copy.testPushNote': '测试推送按钮始终发送固定文案。', 'settings.notifications.copy.permissionRequest': '权限请求', 'settings.notifications.copy.ready': '会话就绪', diff --git a/web/src/routes/settings/notifications.test.tsx b/web/src/routes/settings/notifications.test.tsx index d70b5de2..3cf7ee3c 100644 --- a/web/src/routes/settings/notifications.test.tsx +++ b/web/src/routes/settings/notifications.test.tsx @@ -155,6 +155,16 @@ describe('SettingsNotificationsPage', () => { }) }) + it('reports a rejected notification copy save', async () => { + updateNotificationCopy.mockRejectedValueOnce(new Error('write failed')) + renderPage() + await screen.findByText('Push notification copy') + + fireEvent.click(screen.getByRole('button', { name: 'Save copy' })) + + expect(await screen.findByText('Failed to save notification copy')).toBeTruthy() + }) + it('keeps copy editors collapsed and prefills defaults when opened', async () => { renderPage() const readyRow = await screen.findByRole('button', { name: /Session ready.*Ready for input/ }) diff --git a/web/src/routes/settings/notifications.tsx b/web/src/routes/settings/notifications.tsx index 12e462df..19f11ea6 100644 --- a/web/src/routes/settings/notifications.tsx +++ b/web/src/routes/settings/notifications.tsx @@ -87,6 +87,7 @@ export default function SettingsNotificationsPage() { const [testPushLabel, setTestPushLabel] = useState(null) const [saveError, setSaveError] = useState(null) const [copySaved, setCopySaved] = useState(false) + const [copySaveError, setCopySaveError] = useState(null) const [openCopyBlock, setOpenCopyBlock] = useState(null) const isAdmin = Boolean(token) && getNamespace(token) === 'default' @@ -142,12 +143,20 @@ export default function SettingsNotificationsPage() { if (!api) throw new Error('API unavailable') return await api.updateNotificationCopy(copy) }, + onMutate: () => { + setCopySaveError(null) + }, onSuccess: (data) => { queryClient.setQueryData(queryKeys.notificationCopy, data) setDraft(resolveEffectiveCopy(data.copy, data.defaults)) + setCopySaveError(null) setCopySaved(true) setTimeout(() => setCopySaved(false), 3000) }, + onError: () => { + setCopySaved(false) + setCopySaveError(t('settings.notifications.copy.saveError')) + }, }) const handleToggle = (key: ToggleKey) => (checked: boolean) => { @@ -297,7 +306,7 @@ export default function SettingsNotificationsPage() { })}
- {copySaved ? t('settings.notifications.copy.saved') : ''} + {copySaveError ?? (copySaved ? t('settings.notifications.copy.saved') : '')}