Skip to content

Commit 044fdaf

Browse files
committed
🤖 refactor: make ExtensionMetadataService stateless with atomic writes
Redesigned ExtensionMetadataService to be stateless and use atomic writes. Created reusable atomic write utilities to centralize write-file-atomic usage. Changes: - Created src/utils/atomicWrite.ts with writeFileAtomically() and writeFileAtomicallySync() - ExtensionMetadataService now uses load-modify-save pattern (no in-memory state) - All read methods now async (load from disk each time) - All write methods use atomic writes via writeFileAtomically() - Simplified IpcMain to use regular constructor + initialize() method - Updated Config.ts to use new writeFileAtomicallySync() utility Benefits: - Stateless: no stale data, always fresh from disk - Atomic writes: prevents corruption from crashes/concurrent writes - Reusable: atomic write logic centralized for use across codebase - Simpler: no cache invalidation or state management needed Trade-offs: - Slightly higher disk I/O (acceptable for read-heavy workload) - Simplicity and correctness over performance optimization
1 parent 8170760 commit 044fdaf

File tree

8 files changed

+94
-83
lines changed

8 files changed

+94
-83
lines changed

src/bench/headlessEnvironment.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,8 @@ export async function createHeadlessEnvironment(
104104
const mockIpcMainModule = mockedElectron.ipcMain;
105105
const mockIpcRendererModule = mockedElectron.ipcRenderer;
106106

107-
const ipcMain = await IpcMain.create(config);
107+
const ipcMain = new IpcMain(config);
108+
await ipcMain.initialize();
108109
ipcMain.register(mockIpcMainModule, mockWindow);
109110

110111
const dispose = async () => {

src/config.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,12 @@ import * as path from "path";
33
import * as os from "os";
44
import * as crypto from "crypto";
55
import * as jsonc from "jsonc-parser";
6-
import writeFileAtomic from "write-file-atomic";
76
import type { WorkspaceMetadata, FrontendWorkspaceMetadata } from "./types/workspace";
87
import type { Secret, SecretsConfig } from "./types/secrets";
98
import type { Workspace, ProjectConfig, ProjectsConfig } from "./types/project";
109
import type { RuntimeConfig } from "./types/runtime";
1110
import { DEFAULT_RUNTIME_CONFIG } from "./constants/workspace";
11+
import { writeFileAtomicallySync } from "./utils/atomicWrite";
1212

1313
// Re-export project types from dedicated types file (for preload usage)
1414
export type { Workspace, ProjectConfig, ProjectsConfig };
@@ -81,7 +81,7 @@ export class Config {
8181
projects: Array.from(config.projects.entries()),
8282
};
8383

84-
writeFileAtomic.sync(this.configFile, JSON.stringify(data, null, 2));
84+
writeFileAtomicallySync(this.configFile, JSON.stringify(data, null, 2));
8585
} catch (error) {
8686
console.error("Error saving config:", error);
8787
}
@@ -485,7 +485,7 @@ ${jsonString}`;
485485
fs.mkdirSync(this.rootDir, { recursive: true });
486486
}
487487

488-
writeFileAtomic.sync(this.secretsFile, JSON.stringify(config, null, 2));
488+
writeFileAtomicallySync(this.secretsFile, JSON.stringify(config, null, 2));
489489
} catch (error) {
490490
console.error("Error saving secrets config:", error);
491491
throw error;

src/main-desktop.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,8 @@ async function loadServices(): Promise<void> {
315315
]);
316316
/* eslint-enable no-restricted-syntax */
317317
config = new ConfigClass();
318-
ipcMain = await IpcMainClass.create(config);
318+
ipcMain = new IpcMainClass(config);
319+
await ipcMain.initialize();
319320

320321
loadTokenizerModules().catch((error) => {
321322
console.error("Failed to preload tokenizer modules:", error);

src/main-server.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,8 @@ const httpIpcMain = new HttpIpcMainAdapter(app);
145145
(async () => {
146146
// Initialize config and IPC service
147147
const config = new Config();
148-
const ipcMainService = await IpcMain.create(config);
148+
const ipcMainService = new IpcMain(config);
149+
await ipcMainService.initialize();
149150

150151
// Register IPC handlers
151152
ipcMainService.register(
Lines changed: 51 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
import { dirname } from "path";
2-
import { mkdir, readFile, writeFile } from "fs/promises";
2+
import { mkdir, readFile } from "fs/promises";
33
import { existsSync } from "fs";
44
import {
55
type ExtensionMetadata,
66
type ExtensionMetadataFile,
77
getExtensionMetadataPath,
88
} from "@/utils/extensionMetadata";
9+
import { writeFileAtomically } from "@/utils/atomicWrite";
910

1011
/**
11-
* Service for managing workspace metadata used by VS Code extension integration.
12+
* Stateless service for managing workspace metadata used by VS Code extension integration.
1213
*
1314
* This service tracks:
1415
* - recency: Unix timestamp (ms) of last user interaction
@@ -17,8 +18,10 @@ import {
1718
*
1819
* File location: ~/.cmux/extensionMetadata.json
1920
*
20-
* Uses atomic writes to prevent corruption. Read-heavy workload (extension reads,
21-
* main app writes on user interactions).
21+
* Design:
22+
* - Stateless: reads from disk on every operation, no in-memory cache
23+
* - Atomic writes: uses write-file-atomic to prevent corruption
24+
* - Read-heavy workload: extension reads, main app writes on user interactions
2225
*/
2326

2427
export interface WorkspaceMetadata extends ExtensionMetadata {
@@ -28,44 +31,33 @@ export interface WorkspaceMetadata extends ExtensionMetadata {
2831

2932
export class ExtensionMetadataService {
3033
private readonly filePath: string;
31-
private data: ExtensionMetadataFile;
3234

33-
private constructor(filePath: string, data: ExtensionMetadataFile) {
34-
this.filePath = filePath;
35-
this.data = data;
35+
constructor(filePath?: string) {
36+
this.filePath = filePath ?? getExtensionMetadataPath();
3637
}
3738

3839
/**
39-
* Create a new ExtensionMetadataService instance.
40-
* Use this static factory method instead of the constructor.
40+
* Initialize the service by ensuring directory exists and clearing stale streaming flags.
41+
* Call this once on app startup.
4142
*/
42-
static async create(filePath?: string): Promise<ExtensionMetadataService> {
43-
const path = filePath ?? getExtensionMetadataPath();
44-
43+
async initialize(): Promise<void> {
4544
// Ensure directory exists
46-
const dir = dirname(path);
45+
const dir = dirname(this.filePath);
4746
if (!existsSync(dir)) {
4847
await mkdir(dir, { recursive: true });
4948
}
5049

51-
// Load existing data or initialize
52-
const data = await ExtensionMetadataService.loadData(path);
53-
54-
const service = new ExtensionMetadataService(path, data);
55-
5650
// Clear stale streaming flags (from crashes)
57-
await service.clearStaleStreaming();
58-
59-
return service;
51+
await this.clearStaleStreaming();
6052
}
6153

62-
private static async loadData(filePath: string): Promise<ExtensionMetadataFile> {
63-
if (!existsSync(filePath)) {
54+
private async load(): Promise<ExtensionMetadataFile> {
55+
if (!existsSync(this.filePath)) {
6456
return { version: 1, workspaces: {} };
6557
}
6658

6759
try {
68-
const content = await readFile(filePath, "utf-8");
60+
const content = await readFile(this.filePath, "utf-8");
6961
const parsed = JSON.parse(content) as ExtensionMetadataFile;
7062

7163
// Validate structure
@@ -83,10 +75,10 @@ export class ExtensionMetadataService {
8375
}
8476
}
8577

86-
private async save(): Promise<void> {
78+
private async save(data: ExtensionMetadataFile): Promise<void> {
8779
try {
88-
const content = JSON.stringify(this.data, null, 2);
89-
await writeFile(this.filePath, content, "utf-8");
80+
const content = JSON.stringify(data, null, 2);
81+
await writeFileAtomically(this.filePath, content);
9082
} catch (error) {
9183
console.error("[ExtensionMetadataService] Failed to save metadata:", error);
9284
}
@@ -97,44 +89,51 @@ export class ExtensionMetadataService {
9789
* Call this on user messages or other interactions.
9890
*/
9991
async updateRecency(workspaceId: string, timestamp: number = Date.now()): Promise<void> {
100-
if (!this.data.workspaces[workspaceId]) {
101-
this.data.workspaces[workspaceId] = {
92+
const data = await this.load();
93+
94+
if (!data.workspaces[workspaceId]) {
95+
data.workspaces[workspaceId] = {
10296
recency: timestamp,
10397
streaming: false,
10498
lastModel: null,
10599
};
106100
} else {
107-
this.data.workspaces[workspaceId].recency = timestamp;
101+
data.workspaces[workspaceId].recency = timestamp;
108102
}
109-
await this.save();
103+
104+
await this.save(data);
110105
}
111106

112107
/**
113108
* Set the streaming status for a workspace.
114109
* Call this when streams start/end.
115110
*/
116111
async setStreaming(workspaceId: string, streaming: boolean, model?: string): Promise<void> {
112+
const data = await this.load();
117113
const now = Date.now();
118-
if (!this.data.workspaces[workspaceId]) {
119-
this.data.workspaces[workspaceId] = {
114+
115+
if (!data.workspaces[workspaceId]) {
116+
data.workspaces[workspaceId] = {
120117
recency: now,
121118
streaming,
122119
lastModel: model ?? null,
123120
};
124121
} else {
125-
this.data.workspaces[workspaceId].streaming = streaming;
122+
data.workspaces[workspaceId].streaming = streaming;
126123
if (model) {
127-
this.data.workspaces[workspaceId].lastModel = model;
124+
data.workspaces[workspaceId].lastModel = model;
128125
}
129126
}
130-
await this.save();
127+
128+
await this.save(data);
131129
}
132130

133131
/**
134132
* Get metadata for a single workspace.
135133
*/
136-
getMetadata(workspaceId: string): WorkspaceMetadata | null {
137-
const entry = this.data.workspaces[workspaceId];
134+
async getMetadata(workspaceId: string): Promise<WorkspaceMetadata | null> {
135+
const data = await this.load();
136+
const entry = data.workspaces[workspaceId];
138137
if (!entry) return null;
139138

140139
return {
@@ -148,11 +147,12 @@ export class ExtensionMetadataService {
148147
* Get all workspace metadata, ordered by recency.
149148
* Used by VS Code extension to sort workspace list.
150149
*/
151-
getAllMetadata(): Map<string, WorkspaceMetadata> {
150+
async getAllMetadata(): Promise<Map<string, WorkspaceMetadata>> {
151+
const data = await this.load();
152152
const map = new Map<string, WorkspaceMetadata>();
153153

154154
// Convert to array, sort by recency, then create map
155-
const entries = Object.entries(this.data.workspaces);
155+
const entries = Object.entries(data.workspaces);
156156
entries.sort((a, b) => b[1].recency - a[1].recency);
157157

158158
for (const [workspaceId, entry] of entries) {
@@ -171,9 +171,11 @@ export class ExtensionMetadataService {
171171
* Call this when a workspace is deleted.
172172
*/
173173
async deleteWorkspace(workspaceId: string): Promise<void> {
174-
if (this.data.workspaces[workspaceId]) {
175-
delete this.data.workspaces[workspaceId];
176-
await this.save();
174+
const data = await this.load();
175+
176+
if (data.workspaces[workspaceId]) {
177+
delete data.workspaces[workspaceId];
178+
await this.save(data);
177179
}
178180
}
179181

@@ -182,15 +184,18 @@ export class ExtensionMetadataService {
182184
* Call this on app startup to clean up stale streaming states from crashes.
183185
*/
184186
async clearStaleStreaming(): Promise<void> {
187+
const data = await this.load();
185188
let modified = false;
186-
for (const entry of Object.values(this.data.workspaces)) {
189+
190+
for (const entry of Object.values(data.workspaces)) {
187191
if (entry.streaming) {
188192
entry.streaming = false;
189193
modified = true;
190194
}
191195
}
196+
192197
if (modified) {
193-
await this.save();
198+
await this.save(data);
194199
}
195200
}
196201
}

src/services/ipcMain.ts

Lines changed: 15 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -59,44 +59,29 @@ export class IpcMain {
5959

6060
private registered = false;
6161

62-
private constructor(
63-
config: Config,
64-
historyService: HistoryService,
65-
partialService: PartialService,
66-
initStateManager: InitStateManager,
67-
extensionMetadata: ExtensionMetadataService,
68-
aiService: AIService
69-
) {
62+
constructor(config: Config) {
7063
this.config = config;
71-
this.historyService = historyService;
72-
this.partialService = partialService;
73-
this.initStateManager = initStateManager;
74-
this.extensionMetadata = extensionMetadata;
75-
this.aiService = aiService;
64+
this.historyService = new HistoryService(config);
65+
this.partialService = new PartialService(config, this.historyService);
66+
this.initStateManager = new InitStateManager(config);
67+
this.extensionMetadata = new ExtensionMetadataService();
68+
this.aiService = new AIService(
69+
config,
70+
this.historyService,
71+
this.partialService,
72+
this.initStateManager
73+
);
7674

7775
// Listen to AIService events to update metadata
7876
this.setupMetadataListeners();
7977
}
8078

8179
/**
82-
* Create a new IpcMain instance.
83-
* Use this static factory method instead of the constructor.
80+
* Initialize the service. Call this after construction.
81+
* This is separate from the constructor to support async initialization.
8482
*/
85-
static async create(config: Config): Promise<IpcMain> {
86-
const historyService = new HistoryService(config);
87-
const partialService = new PartialService(config, historyService);
88-
const initStateManager = new InitStateManager(config);
89-
const extensionMetadata = await ExtensionMetadataService.create();
90-
const aiService = new AIService(config, historyService, partialService, initStateManager);
91-
92-
return new IpcMain(
93-
config,
94-
historyService,
95-
partialService,
96-
initStateManager,
97-
extensionMetadata,
98-
aiService
99-
);
83+
async initialize(): Promise<void> {
84+
await this.extensionMetadata.initialize();
10085
}
10186

10287
/**

src/utils/atomicWrite.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import writeFileAtomic from "write-file-atomic";
2+
3+
/**
4+
* Atomically write data to a file (async).
5+
* Uses write-file-atomic to ensure the file is never in a half-written state.
6+
*/
7+
export async function writeFileAtomically(filePath: string, data: string): Promise<void> {
8+
await writeFileAtomic(filePath, data, "utf-8");
9+
}
10+
11+
/**
12+
* Atomically write data to a file (sync).
13+
* Uses write-file-atomic to ensure the file is never in a half-written state.
14+
*/
15+
export function writeFileAtomicallySync(filePath: string, data: string): void {
16+
writeFileAtomic.sync(filePath, data);
17+
}

tests/ipcMain/setup.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ export async function createTestEnvironment(): Promise<TestEnvironment> {
6666
const mockIpcRendererModule = mocked.ipcRenderer;
6767

6868
// Create IpcMain instance
69-
const ipcMain = await IpcMain.create(config);
69+
const ipcMain = new IpcMain(config);
70+
await ipcMain.initialize();
7071

7172
// Register handlers with mock ipcMain and window
7273
ipcMain.register(mockIpcMainModule, mockWindow);

0 commit comments

Comments
 (0)