Skip to content

Commit e3ea765

Browse files
authored
Normalize vfs (#558)
1 parent 8260aa8 commit e3ea765

File tree

2 files changed

+105
-116
lines changed

2 files changed

+105
-116
lines changed

e2e-tests/problems.spec.ts

Lines changed: 31 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
import { test, testSkipIfWindows } from "./helpers/test_helper";
1+
import { test } from "./helpers/test_helper";
22
import { expect } from "@playwright/test";
33
import fs from "fs";
44
import path from "path";
55

66
const MINIMAL_APP = "minimal-with-ai-rules";
77

8-
testSkipIfWindows("problems auto-fix - enabled", async ({ po }) => {
8+
test("problems auto-fix - enabled", async ({ po }) => {
99
await po.setUp();
1010
await po.importApp(MINIMAL_APP);
1111
await po.expectPreviewIframeIsVisible();
@@ -18,41 +18,35 @@ testSkipIfWindows("problems auto-fix - enabled", async ({ po }) => {
1818
await po.snapshotMessages({ replaceDumpPath: true });
1919
});
2020

21-
testSkipIfWindows(
22-
"problems auto-fix - gives up after 2 attempts",
23-
async ({ po }) => {
24-
await po.setUp();
25-
await po.importApp(MINIMAL_APP);
26-
await po.expectPreviewIframeIsVisible();
27-
28-
await po.sendPrompt("tc=create-unfixable-ts-errors");
29-
30-
await po.snapshotServerDump("all-messages", { dumpIndex: -2 });
31-
await po.snapshotServerDump("all-messages", { dumpIndex: -1 });
32-
33-
await po.page.getByTestId("problem-summary").last().click();
34-
await expect(
35-
po.page.getByTestId("problem-summary").last(),
36-
).toMatchAriaSnapshot();
37-
await po.snapshotMessages({ replaceDumpPath: true });
38-
},
39-
);
40-
41-
testSkipIfWindows(
42-
"problems auto-fix - complex delete-rename-write",
43-
async ({ po }) => {
44-
await po.setUp();
45-
await po.importApp(MINIMAL_APP);
46-
await po.expectPreviewIframeIsVisible();
47-
48-
await po.sendPrompt("tc=create-ts-errors-complex");
49-
50-
await po.snapshotServerDump("all-messages", { dumpIndex: -2 });
51-
await po.snapshotServerDump("all-messages", { dumpIndex: -1 });
52-
53-
await po.snapshotMessages({ replaceDumpPath: true });
54-
},
55-
);
21+
test("problems auto-fix - gives up after 2 attempts", async ({ po }) => {
22+
await po.setUp();
23+
await po.importApp(MINIMAL_APP);
24+
await po.expectPreviewIframeIsVisible();
25+
26+
await po.sendPrompt("tc=create-unfixable-ts-errors");
27+
28+
await po.snapshotServerDump("all-messages", { dumpIndex: -2 });
29+
await po.snapshotServerDump("all-messages", { dumpIndex: -1 });
30+
31+
await po.page.getByTestId("problem-summary").last().click();
32+
await expect(
33+
po.page.getByTestId("problem-summary").last(),
34+
).toMatchAriaSnapshot();
35+
await po.snapshotMessages({ replaceDumpPath: true });
36+
});
37+
38+
test("problems auto-fix - complex delete-rename-write", async ({ po }) => {
39+
await po.setUp();
40+
await po.importApp(MINIMAL_APP);
41+
await po.expectPreviewIframeIsVisible();
42+
43+
await po.sendPrompt("tc=create-ts-errors-complex");
44+
45+
await po.snapshotServerDump("all-messages", { dumpIndex: -2 });
46+
await po.snapshotServerDump("all-messages", { dumpIndex: -1 });
47+
48+
await po.snapshotMessages({ replaceDumpPath: true });
49+
});
5650

5751
test("problems auto-fix - disabled", async ({ po }) => {
5852
await po.setUp({ disableAutoFixProblems: true });

src/utils/VirtualFilesystem.ts

Lines changed: 74 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
getDyadRenameTags,
66
getDyadDeleteTags,
77
} from "../ipc/processors/response_processor";
8+
import { normalizePath } from "../ipc/processors/normalizePath";
89

910
import log from "electron-log";
1011

@@ -39,7 +40,39 @@ export abstract class BaseVirtualFileSystem {
3940
protected baseDir: string;
4041

4142
constructor(baseDir: string) {
42-
this.baseDir = baseDir;
43+
this.baseDir = path.resolve(baseDir);
44+
}
45+
46+
/**
47+
* Normalize path for consistent cross-platform behavior
48+
*/
49+
private normalizePathForKey(filePath: string): string {
50+
const absolutePath = path.isAbsolute(filePath)
51+
? filePath
52+
: path.resolve(this.baseDir, filePath);
53+
54+
// Normalize separators and handle case-insensitive Windows paths
55+
const normalized = normalizePath(path.normalize(absolutePath));
56+
57+
// Intentionally do NOT lowercase for Windows which is case-insensitive
58+
// because this avoids issues with path comparison.
59+
//
60+
// This is a trade-off and introduces a small edge case where
61+
// e.g. foo.txt and Foo.txt are treated as different files by the VFS
62+
// even though Windows treats them as the same file.
63+
//
64+
// This should be a pretty rare occurence and it's not worth the extra
65+
// complexity to handle it.
66+
return normalized;
67+
}
68+
69+
/**
70+
* Convert normalized path back to platform-appropriate format
71+
*/
72+
private denormalizePath(normalizedPath: string): string {
73+
return process.platform === "win32"
74+
? normalizedPath.replace(/\//g, "\\")
75+
: normalizedPath;
4376
}
4477

4578
/**
@@ -69,43 +102,49 @@ export abstract class BaseVirtualFileSystem {
69102
/**
70103
* Write a file to the virtual filesystem
71104
*/
72-
public writeFile(relativePath: string, content: string): void {
105+
protected writeFile(relativePath: string, content: string): void {
73106
const absolutePath = path.resolve(this.baseDir, relativePath);
74-
this.virtualFiles.set(absolutePath, content);
107+
const normalizedKey = this.normalizePathForKey(absolutePath);
108+
109+
this.virtualFiles.set(normalizedKey, content);
75110
// Remove from deleted files if it was previously deleted
76-
this.deletedFiles.delete(absolutePath);
111+
this.deletedFiles.delete(normalizedKey);
77112
}
78113

79114
/**
80115
* Delete a file from the virtual filesystem
81116
*/
82-
public deleteFile(relativePath: string): void {
117+
protected deleteFile(relativePath: string): void {
83118
const absolutePath = path.resolve(this.baseDir, relativePath);
84-
this.deletedFiles.add(absolutePath);
119+
const normalizedKey = this.normalizePathForKey(absolutePath);
120+
121+
this.deletedFiles.add(normalizedKey);
85122
// Remove from virtual files if it exists there
86-
this.virtualFiles.delete(absolutePath);
123+
this.virtualFiles.delete(normalizedKey);
87124
}
88125

89126
/**
90127
* Rename a file in the virtual filesystem
91128
*/
92-
public renameFile(fromPath: string, toPath: string): void {
129+
protected renameFile(fromPath: string, toPath: string): void {
93130
const fromAbsolute = path.resolve(this.baseDir, fromPath);
94131
const toAbsolute = path.resolve(this.baseDir, toPath);
132+
const fromNormalized = this.normalizePathForKey(fromAbsolute);
133+
const toNormalized = this.normalizePathForKey(toAbsolute);
95134

96135
// Mark old file as deleted
97-
this.deletedFiles.add(fromAbsolute);
136+
this.deletedFiles.add(fromNormalized);
98137

99138
// If the source file exists in virtual files, move its content
100-
if (this.virtualFiles.has(fromAbsolute)) {
101-
const content = this.virtualFiles.get(fromAbsolute)!;
102-
this.virtualFiles.delete(fromAbsolute);
103-
this.virtualFiles.set(toAbsolute, content);
139+
if (this.virtualFiles.has(fromNormalized)) {
140+
const content = this.virtualFiles.get(fromNormalized)!;
141+
this.virtualFiles.delete(fromNormalized);
142+
this.virtualFiles.set(toNormalized, content);
104143
} else {
105144
// Try to read from actual filesystem
106145
try {
107146
const content = fs.readFileSync(fromAbsolute, "utf8");
108-
this.virtualFiles.set(toAbsolute, content);
147+
this.virtualFiles.set(toNormalized, content);
109148
} catch (error) {
110149
// If we can't read the source file, we'll let the consumer handle it
111150
logger.warn(
@@ -116,103 +155,59 @@ export abstract class BaseVirtualFileSystem {
116155
}
117156

118157
// Remove destination from deleted files if it was previously deleted
119-
this.deletedFiles.delete(toAbsolute);
158+
this.deletedFiles.delete(toNormalized);
120159
}
121160

122161
/**
123162
* Get all virtual files (files that have been written or modified)
124163
*/
125164
public getVirtualFiles(): VirtualFile[] {
126165
return Array.from(this.virtualFiles.entries()).map(
127-
([absolutePath, content]) => ({
128-
path: path.relative(this.baseDir, absolutePath),
129-
content,
130-
}),
166+
([normalizedKey, content]) => {
167+
// Convert normalized key back to relative path
168+
const denormalizedPath = this.denormalizePath(normalizedKey);
169+
170+
return {
171+
path: path.relative(this.baseDir, denormalizedPath),
172+
content,
173+
};
174+
},
131175
);
132176
}
133177

134178
/**
135179
* Get all deleted file paths (relative to base directory)
136180
*/
137181
public getDeletedFiles(): string[] {
138-
return Array.from(this.deletedFiles).map((absolutePath) =>
139-
path.relative(this.baseDir, absolutePath),
140-
);
141-
}
142-
143-
/**
144-
* Get all files that should be considered (existing + virtual - deleted)
145-
*/
146-
public getAllFiles(): string[] {
147-
const allFiles = new Set<string>();
148-
149-
// Add virtual files
150-
for (const [absolutePath] of this.virtualFiles.entries()) {
151-
allFiles.add(path.relative(this.baseDir, absolutePath));
152-
}
153-
154-
// Add existing files (this is a simplified version - in practice you might want to scan the directory)
155-
// This method is mainly for getting the current state, consumers can combine with directory scanning
156-
157-
return Array.from(allFiles);
158-
}
159-
160-
/**
161-
* Check if a file has been modified in the virtual filesystem
162-
*/
163-
public isFileModified(filePath: string): boolean {
164-
const absolutePath = path.isAbsolute(filePath)
165-
? filePath
166-
: path.resolve(this.baseDir, filePath);
167-
168-
return (
169-
this.virtualFiles.has(absolutePath) || this.deletedFiles.has(absolutePath)
170-
);
171-
}
172-
173-
/**
174-
* Clear all virtual changes
175-
*/
176-
public clear(): void {
177-
this.virtualFiles.clear();
178-
this.deletedFiles.clear();
179-
}
180-
181-
/**
182-
* Get the base directory
183-
*/
184-
public getBaseDir(): string {
185-
return this.baseDir;
182+
return Array.from(this.deletedFiles).map((normalizedKey) => {
183+
// Convert normalized key back to relative path
184+
const denormalizedPath = this.denormalizePath(normalizedKey);
185+
return path.relative(this.baseDir, denormalizedPath);
186+
});
186187
}
187188

188189
/**
189190
* Check if a file is deleted in the virtual filesystem
190191
*/
191192
protected isDeleted(filePath: string): boolean {
192-
const absolutePath = path.isAbsolute(filePath)
193-
? filePath
194-
: path.resolve(this.baseDir, filePath);
195-
return this.deletedFiles.has(absolutePath);
193+
const normalizedKey = this.normalizePathForKey(filePath);
194+
return this.deletedFiles.has(normalizedKey);
196195
}
197196

198197
/**
199198
* Check if a file exists in virtual files
200199
*/
201200
protected hasVirtualFile(filePath: string): boolean {
202-
const absolutePath = path.isAbsolute(filePath)
203-
? filePath
204-
: path.resolve(this.baseDir, filePath);
205-
return this.virtualFiles.has(absolutePath);
201+
const normalizedKey = this.normalizePathForKey(filePath);
202+
return this.virtualFiles.has(normalizedKey);
206203
}
207204

208205
/**
209206
* Get virtual file content
210207
*/
211208
protected getVirtualFileContent(filePath: string): string | undefined {
212-
const absolutePath = path.isAbsolute(filePath)
213-
? filePath
214-
: path.resolve(this.baseDir, filePath);
215-
return this.virtualFiles.get(absolutePath);
209+
const normalizedKey = this.normalizePathForKey(filePath);
210+
return this.virtualFiles.get(normalizedKey);
216211
}
217212
}
218213

0 commit comments

Comments
 (0)