Skip to content

Commit 89c5d87

Browse files
iainRedroiainRedro
committed
Refine for PR (Fix mode state sharing when switching between open vscode windows)
1 parent b818f6b commit 89c5d87

File tree

2 files changed

+136
-105
lines changed

2 files changed

+136
-105
lines changed

‎src/core/__tests__/contextProxy.test.ts‎

Lines changed: 79 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ jest.mock("vscode", () => ({
2121
},
2222
env: {
2323
sessionId: "test-session-id",
24-
machineId: "test-machine-id"
24+
machineId: "test-machine-id",
2525
},
2626
workspace: {
2727
name: "test-workspace-name",
@@ -130,9 +130,35 @@ describe("ContextProxy", () => {
130130
expect(mockGlobalState.update).toHaveBeenCalledWith("test-key", "new-value")
131131

132132
// Should have stored the value in cache
133-
const storedValue = await proxy.getGlobalState("test-key")
133+
const storedValue = proxy.getGlobalState("test-key")
134134
expect(storedValue).toBe("new-value")
135135
})
136+
137+
it("should handle window-specific keys correctly", async () => {
138+
// Test with a window-specific key
139+
await proxy.updateGlobalState("mode", "test-mode")
140+
141+
// Should have called update with window-specific key
142+
expect(mockGlobalState.update).toHaveBeenCalledWith(expect.stringMatching(/^window:.+:mode$/), "test-mode")
143+
144+
// Should have stored the value in cache with the original key
145+
const storedValue = proxy.getGlobalState("mode")
146+
expect(storedValue).toBe("test-mode")
147+
})
148+
149+
it("should throw and not update cache if storage update fails", async () => {
150+
// Mock a failure in the storage update
151+
mockGlobalState.update.mockRejectedValueOnce(new Error("Storage update failed"))
152+
153+
// Set initial cache value
154+
proxy["stateCache"].set("error-key", "initial-value")
155+
156+
// Attempt to update should fail
157+
await expect(proxy.updateGlobalState("error-key", "new-value")).rejects.toThrow("Storage update failed")
158+
159+
// Cache should still have the initial value
160+
expect(proxy.getGlobalState("error-key")).toBe("initial-value")
161+
})
136162
})
137163

138164
describe("getSecret", () => {
@@ -356,22 +382,22 @@ describe("ContextProxy", () => {
356382
it("should use window-specific key for mode", async () => {
357383
// Ensure 'mode' is in window specific keys
358384
expect(WINDOW_SPECIFIC_KEYS).toContain("mode")
359-
385+
360386
// Test update method with 'mode' key
361387
await proxy.updateGlobalState("mode", "debug")
362-
388+
363389
// Verify it's called with window-specific key
364390
expect(mockGlobalState.update).toHaveBeenCalledWith(expect.stringMatching(/^window:.+:mode$/), "debug")
365391
})
366-
392+
367393
it("should use regular key for non-window-specific state", async () => {
368394
// Test update method with a regular key
369395
await proxy.updateGlobalState("apiProvider", "test-provider")
370-
396+
371397
// Verify it's called with regular key
372398
expect(mockGlobalState.update).toHaveBeenCalledWith("apiProvider", "test-provider")
373399
})
374-
400+
375401
it("should consistently use same key format for get/update operations", async () => {
376402
// Set mock values for testing
377403
const windowKeyPattern = /^window:.+:mode$/
@@ -380,20 +406,20 @@ describe("ContextProxy", () => {
380406
if (key === "mode") return "global-debug-mode"
381407
return undefined
382408
})
383-
409+
384410
// Update a window-specific value
385411
await proxy.updateGlobalState("mode", "test-mode")
386-
412+
387413
// The key used in update should match pattern
388414
const updateCallArg = mockGlobalState.update.mock.calls[0][0]
389415
expect(updateCallArg).toMatch(windowKeyPattern)
390-
416+
391417
// Re-init to load values
392418
proxy["initializeStateCache"]()
393-
419+
394420
// Verify we get the window-specific value back
395421
const value = proxy.getGlobalState("mode")
396-
422+
397423
// We should get the window-specific value, not the global one
398424
expect(mockGlobalState.get).toHaveBeenCalledWith(expect.stringMatching(windowKeyPattern))
399425
expect(value).not.toBe("global-debug-mode")
@@ -403,61 +429,62 @@ describe("ContextProxy", () => {
403429
describe("Enhanced window ID generation", () => {
404430
it("should generate a window ID that includes workspace name", () => {
405431
// Access the private method using type assertion
406-
const generateWindowId = (proxy as any).generateWindowId.bind(proxy);
407-
const windowId = generateWindowId();
408-
432+
const generateWindowId = (proxy as any).generateWindowId.bind(proxy)
433+
const windowId = generateWindowId()
434+
409435
// Should include the workspace name from our mock
410-
expect(windowId).toContain("test-workspace-name");
411-
});
412-
436+
expect(windowId).toContain("test-workspace-name")
437+
})
438+
413439
it("should generate a window ID that includes machine ID", () => {
414440
// Access the private method using type assertion
415-
const generateWindowId = (proxy as any).generateWindowId.bind(proxy);
416-
const windowId = generateWindowId();
417-
441+
const generateWindowId = (proxy as any).generateWindowId.bind(proxy)
442+
const windowId = generateWindowId()
443+
418444
// Should include the machine ID from our mock
419-
expect(windowId).toContain("test-machine-id");
420-
});
421-
445+
expect(windowId).toContain("test-machine-id")
446+
})
447+
422448
it("should use the fallback mechanism if generateWindowId fails", () => {
423449
// Create a proxy instance with a failing generateWindowId method
424-
const spyOnGenerate = jest.spyOn(ContextProxy.prototype as any, "generateWindowId")
425-
.mockImplementation(() => "");
426-
450+
const spyOnGenerate = jest
451+
.spyOn(ContextProxy.prototype as any, "generateWindowId")
452+
.mockImplementation(() => "")
453+
427454
// Create a new proxy to trigger the constructor with our mock
428-
const testProxy = new ContextProxy(mockContext);
429-
455+
const testProxy = new ContextProxy(mockContext)
456+
430457
// Should have called ensureUniqueWindowId with a fallback
431-
expect(spyOnGenerate).toHaveBeenCalled();
432-
458+
expect(spyOnGenerate).toHaveBeenCalled()
459+
433460
// The windowId should use the fallback format (random ID)
434461
// We can't test the exact value, but we can verify it's not empty
435-
expect((testProxy as any).windowId).not.toBe("");
436-
462+
expect((testProxy as any).windowId).not.toBe("")
463+
437464
// Restore original implementation
438-
spyOnGenerate.mockRestore();
439-
});
440-
465+
spyOnGenerate.mockRestore()
466+
})
467+
441468
it("should create consistent session hash for same input", () => {
442469
// Access the private method using type assertion
443-
const createSessionHash = (proxy as any).createSessionHash.bind(proxy);
444-
445-
const hash1 = createSessionHash("test-input");
446-
const hash2 = createSessionHash("test-input");
447-
470+
const createSessionHash = (proxy as any).createSessionHash.bind(proxy)
471+
472+
const hash1 = createSessionHash("test-input")
473+
const hash2 = createSessionHash("test-input")
474+
448475
// Same input should produce same hash within the same session
449-
expect(hash1).toBe(hash2);
450-
});
451-
476+
expect(hash1).toBe(hash2)
477+
})
478+
452479
it("should create different session hashes for different inputs", () => {
453480
// Access the private method using type assertion
454-
const createSessionHash = (proxy as any).createSessionHash.bind(proxy);
455-
456-
const hash1 = createSessionHash("test-input-1");
457-
const hash2 = createSessionHash("test-input-2");
458-
481+
const createSessionHash = (proxy as any).createSessionHash.bind(proxy)
482+
483+
const hash1 = createSessionHash("test-input-1")
484+
const hash2 = createSessionHash("test-input-2")
485+
459486
// Different inputs should produce different hashes
460-
expect(hash1).not.toBe(hash2);
461-
});
462-
});
487+
expect(hash1).not.toBe(hash2)
488+
})
489+
})
463490
})

‎src/core/contextProxy.ts‎

Lines changed: 57 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ export class ContextProxy {
1818
this.originalContext = context
1919
this.stateCache = new Map()
2020
this.secretCache = new Map()
21-
21+
2222
// Generate a unique ID for this window instance
2323
this.windowId = this.ensureUniqueWindowId()
2424
logger.debug(`ContextProxy created with windowId: ${this.windowId}`)
@@ -38,16 +38,16 @@ export class ContextProxy {
3838
*/
3939
private ensureUniqueWindowId(): string {
4040
// Try to get a stable ID first
41-
let id = this.generateWindowId();
42-
41+
let id = this.generateWindowId()
42+
4343
// If all else fails, use a purely random ID as ultimate fallback
4444
// This will not be stable across restarts but ensures uniqueness
4545
if (!id) {
46-
id = `random_${Date.now()}_${Math.random().toString(36).substring(2, 9)}`;
47-
logger.warn("Failed to generate stable window ID, using random ID instead");
46+
id = `random_${Date.now()}_${Math.random().toString(36).substring(2, 9)}`
47+
logger.warn("Failed to generate stable window ID, using random ID instead")
4848
}
49-
50-
return id;
49+
50+
return id
5151
}
5252

5353
/**
@@ -63,27 +63,27 @@ export class ContextProxy {
6363
private generateWindowId(): string {
6464
try {
6565
// Get all available identifying information
66-
const folders = vscode.workspace.workspaceFolders || [];
67-
const workspaceName = vscode.workspace.name || "unknown";
68-
const folderPaths = folders.map(folder => folder.uri.toString()).join('|');
69-
66+
const folders = vscode.workspace.workspaceFolders || []
67+
const workspaceName = vscode.workspace.name || "unknown"
68+
const folderPaths = folders.map((folder) => folder.uri.toString()).join("|")
69+
7070
// Generate a stable, pseudorandom ID based on the workspace information
7171
// This will be consistent for the same workspace but different across workspaces
72-
const baseId = `${workspaceName}|${folderPaths}`;
73-
72+
const baseId = `${workspaceName}|${folderPaths}`
73+
7474
// Add machine-specific information (will differ between host and containers)
7575
// env.machineId is stable across VS Code sessions on the same machine
76-
const machineSpecificId = vscode.env.machineId || "";
77-
76+
const machineSpecificId = vscode.env.machineId || ""
77+
7878
// Add a session component that distinguishes multiple windows with the same workspace
7979
// Creates a stable but reasonably unique hash
80-
const sessionHash = this.createSessionHash(baseId);
81-
80+
const sessionHash = this.createSessionHash(baseId)
81+
8282
// Combine all components
83-
return `${baseId}|${machineSpecificId}|${sessionHash}`;
83+
return `${baseId}|${machineSpecificId}|${sessionHash}`
8484
} catch (error) {
85-
logger.error("Error generating window ID:", error);
86-
return ""; // Empty string triggers the fallback in ensureUniqueWindowId
85+
logger.error("Error generating window ID:", error)
86+
return "" // Empty string triggers the fallback in ensureUniqueWindowId
8787
}
8888
}
8989

@@ -95,26 +95,26 @@ export class ContextProxy {
9595
try {
9696
// Use a combination of:
9797
// 1. The extension instance creation time
98-
const timestamp = this.instanceCreationTime.getTime();
99-
98+
const timestamp = this.instanceCreationTime.getTime()
99+
100100
// 2. VS Code window-specific info we can derive
101101
// Using vscode.env.sessionId which changes on each VS Code window startup
102-
const sessionInfo = vscode.env.sessionId || "";
103-
102+
const sessionInfo = vscode.env.sessionId || ""
103+
104104
// 3. Calculate a simple hash
105-
const hashStr = `${input}|${sessionInfo}|${timestamp}`;
106-
let hash = 0;
105+
const hashStr = `${input}|${sessionInfo}|${timestamp}`
106+
let hash = 0
107107
for (let i = 0; i < hashStr.length; i++) {
108-
const char = hashStr.charCodeAt(i);
109-
hash = ((hash << 5) - hash) + char;
110-
hash = hash & hash; // Convert to 32bit integer
108+
const char = hashStr.charCodeAt(i)
109+
hash = (hash << 5) - hash + char
110+
hash = hash & hash // Convert to 32bit integer
111111
}
112-
112+
113113
// Return a hexadecimal representation
114-
return Math.abs(hash).toString(16).substring(0, 8);
114+
return Math.abs(hash).toString(16).substring(0, 8)
115115
} catch (error) {
116-
logger.error("Error creating session hash:", error);
117-
return Math.random().toString(36).substring(2, 10); // Random fallback
116+
logger.error("Error creating session hash:", error)
117+
return Math.random().toString(36).substring(2, 10) // Random fallback
118118
}
119119
}
120120

@@ -193,29 +193,33 @@ export class ContextProxy {
193193
getGlobalState<T>(key: string): T | undefined
194194
getGlobalState<T>(key: string, defaultValue: T): T
195195
getGlobalState<T>(key: string, defaultValue?: T): T | undefined {
196-
// Check for window-specific key
197-
if (this.isWindowSpecificKey(key)) {
198-
// Use the cached value if it exists (it would have been loaded with the window-specific key)
199-
const value = this.stateCache.get(key) as T | undefined
200-
return value !== undefined ? value : (defaultValue as T | undefined)
201-
} else {
202-
// For regular global keys, use the regular cached value
203-
const value = this.stateCache.get(key) as T | undefined
204-
return value !== undefined ? value : (defaultValue as T | undefined)
205-
}
196+
// The cache already contains the correct value regardless of whether
197+
// this is a window-specific key (handled during initialization and updates)
198+
const value = this.stateCache.get(key) as T | undefined
199+
return value !== undefined ? value : (defaultValue as T | undefined)
206200
}
207201

208-
updateGlobalState<T>(key: string, value: T): Thenable<void> {
209-
// Update in-memory cache
210-
this.stateCache.set(key, value)
202+
async updateGlobalState<T>(key: string, value: T): Promise<void> {
203+
try {
204+
// Determine the storage key
205+
const storageKey = this.isWindowSpecificKey(key) ? this.getWindowSpecificKey(key) : key
211206

212-
// Update in VSCode storage with appropriate key
213-
if (this.isWindowSpecificKey(key)) {
214-
const windowKey = this.getWindowSpecificKey(key)
215-
logger.debug(`Updating window-specific key ${key} as ${windowKey} with value: ${JSON.stringify(value)}`)
216-
return this.originalContext.globalState.update(windowKey, value)
217-
} else {
218-
return this.originalContext.globalState.update(key, value)
207+
if (this.isWindowSpecificKey(key)) {
208+
logger.debug(
209+
`Updating window-specific key ${key} as ${storageKey} with value: ${JSON.stringify(value)}`,
210+
)
211+
}
212+
213+
// Update in VSCode storage first
214+
await this.originalContext.globalState.update(storageKey, value)
215+
216+
// Only update cache if storage update succeeded
217+
this.stateCache.set(key, value)
218+
} catch (error) {
219+
logger.error(
220+
`Failed to update global state for key ${key}: ${error instanceof Error ? error.message : String(error)}`,
221+
)
222+
throw error // Re-throw to allow callers to handle the error
219223
}
220224
}
221225

0 commit comments

Comments
 (0)