fix: pr feedback

This commit is contained in:
galz10
2026-01-27 14:56:12 -08:00
parent c1a3dca497
commit 68135e1a83
3 changed files with 115 additions and 8 deletions

View File

@@ -63,15 +63,30 @@ export interface TrustResult {
source: 'ide' | 'file' | undefined;
}
const realPathCache = new Map<string, string>();
/**
* FOR TESTING PURPOSES ONLY.
* Clears the real path cache.
*/
export function clearRealPathCacheForTesting(): void {
realPathCache.clear();
}
function getRealPath(location: string): string {
try {
if (fs.existsSync(location)) {
return fs.realpathSync(location);
}
} catch (_e) {
// Fallback to unresolved path
let realPath = realPathCache.get(location);
if (realPath !== undefined) {
return realPath;
}
return location;
try {
realPath = fs.existsSync(location) ? fs.realpathSync(location) : location;
} catch {
realPath = location;
}
realPathCache.set(location, realPath);
return realPath;
}
export class LoadedTrustedFolders {
@@ -158,6 +173,7 @@ let loadedTrustedFolders: LoadedTrustedFolders | undefined;
*/
export function resetTrustedFoldersForTesting(): void {
loadedTrustedFolders = undefined;
clearRealPathCacheForTesting();
}
export function loadTrustedFolders(): LoadedTrustedFolders {

View File

@@ -0,0 +1,91 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import * as fs from 'node:fs';
import {
loadTrustedFolders,
TrustLevel,
resetTrustedFoldersForTesting,
} from './trustedFolders.js';
vi.mock('fs', async (importOriginal) => {
const actualFs = await importOriginal<typeof fs>();
return {
...actualFs,
existsSync: vi.fn(),
realpathSync: vi.fn(),
readFileSync: vi.fn(),
};
});
describe('Trusted Folders realpath caching', () => {
beforeEach(() => {
resetTrustedFoldersForTesting();
vi.resetAllMocks();
});
afterEach(() => {
vi.restoreAllMocks();
});
it('should only call fs.realpathSync once for the same path', () => {
const mockPath = '/some/path';
const mockRealPath = '/real/path';
vi.mocked(fs.existsSync).mockReturnValue(true);
vi.mocked(fs.realpathSync).mockReturnValue(mockRealPath);
vi.mocked(fs.readFileSync).mockReturnValue(
JSON.stringify({
[mockPath]: TrustLevel.TRUST_FOLDER,
'/another/path': TrustLevel.TRUST_FOLDER,
}),
);
const folders = loadTrustedFolders();
// Call isPathTrusted multiple times with the same path
folders.isPathTrusted(mockPath);
folders.isPathTrusted(mockPath);
folders.isPathTrusted(mockPath);
// fs.realpathSync should only be called once for mockPath (at the start of isPathTrusted)
// And once for each rule in the config (if they are different)
// Let's check calls for mockPath
const realpathCalls = vi.mocked(fs.realpathSync).mock.calls;
const mockPathCalls = realpathCalls.filter((call) => call[0] === mockPath);
expect(mockPathCalls.length).toBe(1);
});
it('should cache results for rule paths in the loop', () => {
const rulePath = '/rule/path';
const locationPath = '/location/path';
vi.mocked(fs.existsSync).mockReturnValue(true);
vi.mocked(fs.realpathSync).mockImplementation((p) => p as string); // identity for simplicity
vi.mocked(fs.readFileSync).mockReturnValue(
JSON.stringify({
[rulePath]: TrustLevel.TRUST_FOLDER,
}),
);
const folders = loadTrustedFolders();
// First call
folders.isPathTrusted(locationPath);
const firstCallCount = vi.mocked(fs.realpathSync).mock.calls.length;
expect(firstCallCount).toBe(2); // locationPath and rulePath
// Second call with same location and same config
folders.isPathTrusted(locationPath);
const secondCallCount = vi.mocked(fs.realpathSync).mock.calls.length;
// Should still be 2 because both were cached
expect(secondCallCount).toBe(2);
});
});

View File

@@ -213,7 +213,7 @@ export const AppContainer = (props: AppContainerProps) => {
const activeHooks = useHookDisplayState();
const [updateInfo, setUpdateInfo] = useState<UpdateObject | null>(null);
const [isTrustedFolder, setIsTrustedFolder] = useState<boolean | undefined>(
isWorkspaceTrusted(settings.merged).isTrusted,
() => isWorkspaceTrusted(settings.merged).isTrusted,
);
const [queueErrorMessage, setQueueErrorMessage] = useState<string | null>(