From 20e48b3dc0e91fd0984ff1d059169aa6b64a84ce Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Wed, 25 Feb 2026 23:42:17 +0700 Subject: [PATCH] refactor(dashboard): switch droid settings I/O to async fs - replace sync fs calls in compatible CLI JSON helper with fs.promises - make droid diagnostics/raw-settings service + routes async - update unit tests for async save/read paths --- src/web-server/routes/droid-routes.ts | 12 ++-- .../compatible-cli-json-file-service.ts | 59 ++++++++++++------- .../services/droid-dashboard-service.ts | 16 ++--- .../droid-dashboard-service.test.ts | 22 +++---- 4 files changed, 65 insertions(+), 44 deletions(-) diff --git a/src/web-server/routes/droid-routes.ts b/src/web-server/routes/droid-routes.ts index 8cc81950..f18867d6 100644 --- a/src/web-server/routes/droid-routes.ts +++ b/src/web-server/routes/droid-routes.ts @@ -14,9 +14,9 @@ const router = Router(); * GET /api/droid/diagnostics * Dashboard-ready Droid installation + BYOK configuration diagnostics. */ -router.get('/diagnostics', (_req: Request, res: Response): void => { +router.get('/diagnostics', async (_req: Request, res: Response): Promise => { try { - res.json(getDroidDashboardDiagnostics()); + res.json(await getDroidDashboardDiagnostics()); } catch (error) { res.status(500).json({ error: (error as Error).message }); } @@ -26,9 +26,9 @@ router.get('/diagnostics', (_req: Request, res: Response): void => { * GET /api/droid/settings/raw * Raw ~/.factory/settings.json payload for editor. */ -router.get('/settings/raw', (_req: Request, res: Response): void => { +router.get('/settings/raw', async (_req: Request, res: Response): Promise => { try { - res.json(getDroidRawSettings()); + res.json(await getDroidRawSettings()); } catch (error) { res.status(500).json({ error: (error as Error).message }); } @@ -38,7 +38,7 @@ router.get('/settings/raw', (_req: Request, res: Response): void => { * PUT /api/droid/settings/raw * Save raw ~/.factory/settings.json payload from dashboard editor. */ -router.put('/settings/raw', (req: Request, res: Response): void => { +router.put('/settings/raw', async (req: Request, res: Response): Promise => { try { const { rawText, expectedMtime } = req.body ?? {}; @@ -54,7 +54,7 @@ router.put('/settings/raw', (req: Request, res: Response): void => { return; } - res.json(saveDroidRawSettings({ rawText, expectedMtime })); + res.json(await saveDroidRawSettings({ rawText, expectedMtime })); } catch (error) { if (error instanceof DroidRawSettingsValidationError) { res.status(400).json({ error: error.message }); diff --git a/src/web-server/services/compatible-cli-json-file-service.ts b/src/web-server/services/compatible-cli-json-file-service.ts index fe425cf8..eacf3a65 100644 --- a/src/web-server/services/compatible-cli-json-file-service.ts +++ b/src/web-server/services/compatible-cli-json-file-service.ts @@ -1,4 +1,4 @@ -import * as fs from 'fs'; +import { promises as fs } from 'fs'; import * as path from 'path'; export interface JsonFileDiagnostics { @@ -55,12 +55,24 @@ function isObject(value: unknown): value is Record { return typeof value === 'object' && value !== null && !Array.isArray(value); } -export function probeJsonObjectFile( +async function statPath(filePath: string): Promise { + try { + return await fs.lstat(filePath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + return null; + } + throw error; + } +} + +export async function probeJsonObjectFile( filePath: string, label: string, displayPath: string -): JsonFileProbe { - if (!fs.existsSync(filePath)) { +): Promise { + const stat = await statPath(filePath); + if (!stat) { return { diagnostics: { label, @@ -79,7 +91,6 @@ export function probeJsonObjectFile( }; } - const stat = fs.lstatSync(filePath); const diagnostics: JsonFileDiagnostics = { label, path: displayPath, @@ -104,7 +115,7 @@ export function probeJsonObjectFile( } try { - const rawText = fs.readFileSync(filePath, 'utf8'); + const rawText = await fs.readFile(filePath, 'utf8'); try { const parsed = JSON.parse(rawText); if (!isObject(parsed)) { @@ -140,9 +151,9 @@ export function parseJsonObjectText( return parsed; } -export function writeJsonObjectFileAtomic( +export async function writeJsonObjectFileAtomic( input: WriteJsonObjectFileInput -): WriteJsonObjectFileResult { +): Promise { const fileLabel = input.fileLabel || path.basename(input.filePath); const parsed = parseJsonObjectText(input.rawText, fileLabel); const targetPath = input.filePath; @@ -151,10 +162,11 @@ export function writeJsonObjectFileAtomic( const dirMode = input.dirMode ?? 0o700; const fileMode = input.fileMode ?? 0o600; - fs.mkdirSync(targetDir, { recursive: true, mode: dirMode }); + await fs.mkdir(targetDir, { recursive: true, mode: dirMode }); - if (fs.existsSync(targetPath)) { - const stat = fs.lstatSync(targetPath); + const targetStat = await statPath(targetPath); + if (targetStat) { + const stat = targetStat; if (stat.isSymbolicLink()) { throw new Error(`Refusing to write: ${fileLabel} is a symlink.`); } @@ -172,8 +184,9 @@ export function writeJsonObjectFileAtomic( let wroteTemp = false; try { - if (fs.existsSync(tempPath)) { - const tempStat = fs.lstatSync(tempPath); + const existingTempStat = await statPath(tempPath); + if (existingTempStat) { + const tempStat = existingTempStat; if (tempStat.isSymbolicLink()) { throw new Error(`Refusing to write: ${fileLabel}.tmp is a symlink.`); } @@ -182,10 +195,10 @@ export function writeJsonObjectFileAtomic( } } - fs.writeFileSync(tempPath, JSON.stringify(parsed, null, 2) + '\n', { mode: fileMode }); + await fs.writeFile(tempPath, JSON.stringify(parsed, null, 2) + '\n', { mode: fileMode }); wroteTemp = true; - const tempStat = fs.lstatSync(tempPath); + const tempStat = await fs.lstat(tempPath); if (tempStat.isSymbolicLink()) { throw new Error(`Refusing to write: ${fileLabel}.tmp is a symlink.`); } @@ -193,20 +206,26 @@ export function writeJsonObjectFileAtomic( throw new Error(`Refusing to write: ${fileLabel}.tmp is not a regular file.`); } - fs.renameSync(tempPath, targetPath); + await fs.rename(tempPath, targetPath); wroteTemp = false; try { - fs.chmodSync(targetPath, fileMode); + await fs.chmod(targetPath, fileMode); } catch { // Best-effort permission hardening. } - const stat = fs.statSync(targetPath); + const stat = await fs.stat(targetPath); return { mtime: stat.mtimeMs }; } finally { - if (wroteTemp && fs.existsSync(tempPath)) { - fs.unlinkSync(tempPath); + if (wroteTemp) { + try { + await fs.unlink(tempPath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + throw error; + } + } } } } diff --git a/src/web-server/services/droid-dashboard-service.ts b/src/web-server/services/droid-dashboard-service.ts index 8a0e24a1..903c56ab 100644 --- a/src/web-server/services/droid-dashboard-service.ts +++ b/src/web-server/services/droid-dashboard-service.ts @@ -157,18 +157,18 @@ export function summarizeDroidCustomModels(customModelsValue: unknown): DroidByo }; } -export function getDroidDashboardDiagnostics(): DroidDashboardDiagnostics { +export async function getDroidDashboardDiagnostics(): Promise { const paths = resolveDroidConfigPaths(); const binaryPath = detectDroidCli(); const source = process.env.CCS_DROID_PATH ? 'CCS_DROID_PATH' : binaryPath ? 'PATH' : 'missing'; - const settingsProbe = probeJsonObjectFile( + const settingsProbe = await probeJsonObjectFile( paths.settingsPath, 'BYOK settings', paths.settingsDisplayPath ); - const legacyConfigProbe = probeJsonObjectFile( + const legacyConfigProbe = await probeJsonObjectFile( paths.legacyConfigPath, 'Legacy config', paths.legacyConfigDisplayPath @@ -222,9 +222,9 @@ export function getDroidDashboardDiagnostics(): DroidDashboardDiagnostics { }; } -export function getDroidRawSettings(): DroidRawSettingsResponse { +export async function getDroidRawSettings(): Promise { const paths = resolveDroidConfigPaths(); - const settingsProbe = probeJsonObjectFile( + const settingsProbe = await probeJsonObjectFile( paths.settingsPath, 'BYOK settings', paths.settingsDisplayPath @@ -241,13 +241,15 @@ export function getDroidRawSettings(): DroidRawSettingsResponse { }; } -export function saveDroidRawSettings(input: SaveDroidRawSettingsInput): SaveDroidRawSettingsResult { +export async function saveDroidRawSettings( + input: SaveDroidRawSettingsInput +): Promise { const paths = resolveDroidConfigPaths(); if (typeof input.rawText !== 'string') { throw new JsonFileValidationError('rawText must be a string.'); } - const saved = writeJsonObjectFileAtomic({ + const saved = await writeJsonObjectFileAtomic({ filePath: paths.settingsPath, rawText: input.rawText, expectedMtime: input.expectedMtime, diff --git a/tests/unit/web-server/droid-dashboard-service.test.ts b/tests/unit/web-server/droid-dashboard-service.test.ts index fcf2542a..c781785f 100644 --- a/tests/unit/web-server/droid-dashboard-service.test.ts +++ b/tests/unit/web-server/droid-dashboard-service.test.ts @@ -88,8 +88,8 @@ describe('droid-dashboard-service', () => { expect(summary.customModels[0].apiKeyPreview).toBe('***1234'); }); - it('returns raw settings payload for missing settings file', () => { - const raw = getDroidRawSettings(); + it('returns raw settings payload for missing settings file', async () => { + const raw = await getDroidRawSettings(); expect(raw.exists).toBe(false); expect(raw.path).toBe('~/.factory/settings.json'); @@ -97,12 +97,12 @@ describe('droid-dashboard-service', () => { expect(raw.settings).toBeNull(); }); - it('returns parseError when settings.json is invalid JSON', () => { + it('returns parseError when settings.json is invalid JSON', async () => { const settingsDir = path.join(testRoot, '.factory'); fs.mkdirSync(settingsDir, { recursive: true }); fs.writeFileSync(path.join(settingsDir, 'settings.json'), '{ invalid-json'); - const raw = getDroidRawSettings(); + const raw = await getDroidRawSettings(); expect(raw.exists).toBe(true); expect(raw.parseError).toBeString(); @@ -110,8 +110,8 @@ describe('droid-dashboard-service', () => { expect(raw.rawText).toContain('invalid-json'); }); - it('saves valid raw settings content', () => { - const result = saveDroidRawSettings({ + it('saves valid raw settings content', async () => { + const result = await saveDroidRawSettings({ rawText: JSON.stringify({ model: 'custom:test-model', customModels: [], @@ -126,23 +126,23 @@ describe('droid-dashboard-service', () => { expect(written.model).toBe('custom:test-model'); }); - it('rejects invalid JSON while saving raw settings', () => { - expect(() => saveDroidRawSettings({ rawText: '{ invalid-json' })).toThrow( + it('rejects invalid JSON while saving raw settings', async () => { + await expect(saveDroidRawSettings({ rawText: '{ invalid-json' })).rejects.toThrow( DroidRawSettingsValidationError ); }); - it('rejects stale writes with conflict error', () => { + it('rejects stale writes with conflict error', async () => { const settingsDir = path.join(testRoot, '.factory'); fs.mkdirSync(settingsDir, { recursive: true }); const settingsPath = path.join(settingsDir, 'settings.json'); fs.writeFileSync(settingsPath, JSON.stringify({ customModels: [] })); - expect(() => + await expect( saveDroidRawSettings({ rawText: JSON.stringify({ model: 'custom:next', customModels: [] }), expectedMtime: 1, }) - ).toThrow(DroidRawSettingsConflictError); + ).rejects.toThrow(DroidRawSettingsConflictError); }); });