Skip to content

Commit e871e77

Browse files
Fix workspace switch race condition, gh auth detection, and file open perf
Fix three open bugs (#581, #551, #572): 1. Bug #581 - Workspace State Not Preserved When Switching Folders: Move closeBuffersBatch() before new project loading in switchToProject() to prevent race conditions between session save and restore. Old terminal PTY processes are now cleaned up before new ones are spawned. 2. Bug #551 - Athas not detecting that GitHub CLI is authenticated: Parse gh auth status stderr for authentication indicators instead of relying solely on exit code. gh auth status can return exit code 0 even when the token is expired or invalid (see cli/cli#8845). Now checks for 'Logged in to' as positive confirmation and 'not logged in', 'token is invalid', 'Failed to log in' as failure indicators. 3. Bug #572 - Athas Lags on CachyOS / File Opening Performance: Remove per-entry symlink resolution from readDirectoryContents() that caused hundreds of stat syscalls during directory listing. Symlinks are still resolved lazily when files are opened. Also combine binary sniffing and content reading into a single readFile() call, eliminating the double-read that occurred for every file open. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
1 parent 945477e commit e871e77

3 files changed

Lines changed: 76 additions & 70 deletions

File tree

crates/github/src/api.rs

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,31 @@ pub fn github_check_cli_status(app: AppHandle) -> Result<GitHubCliStatus, String
4040
.output()
4141
.map_err(|e| format!("Failed to execute gh command: {}", e))?;
4242

43+
// gh auth status reports results on stderr. On some versions, exit code 0
44+
// is returned even when the token is expired or invalid (see cli/cli#8845).
45+
// Parse stderr to look for authentication failure indicators instead of
46+
// relying solely on the exit code.
47+
let stderr = String::from_utf8_lossy(&output.stderr);
48+
4349
if output.status.success() {
44-
Ok(GitHubCliStatus::Authenticated)
50+
// Exit code 0, but verify the output actually confirms authentication.
51+
// A successful auth status contains "Logged in to" on stderr.
52+
if stderr.contains("Logged in to") {
53+
Ok(GitHubCliStatus::Authenticated)
54+
} else if stderr.contains("not logged in")
55+
|| stderr.contains("no authentications")
56+
|| stderr.contains("token is invalid")
57+
|| stderr.contains("Failed to log in")
58+
|| (stderr.contains("The token") && stderr.contains("is invalid"))
59+
{
60+
// Token exists but is invalid/expired — gh still exits 0 in some versions
61+
Ok(GitHubCliStatus::NotAuthenticated)
62+
} else {
63+
// Exit code 0 with no recognizable failure — assume authenticated
64+
Ok(GitHubCliStatus::Authenticated)
65+
}
4566
} else {
67+
// Non-zero exit code always means not authenticated
4668
Ok(GitHubCliStatus::NotAuthenticated)
4769
}
4870
}

src/features/file-system/controllers/file-operations.ts

Lines changed: 13 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
11
import type { FileEntry } from "../types/app";
22
import {
3-
getSymlinkInfo,
43
createDirectory as platformCreateDirectory,
54
deletePath as platformDeletePath,
65
readDirectory as platformReadDirectory,
76
readFile as platformReadFile,
87
writeFile as platformWriteFile,
98
} from "./platform";
10-
import { useFileSystemStore } from "./store";
119
import { shouldIgnore } from "./utils";
1210

1311
export async function readFileContent(path: string): Promise<string> {
@@ -60,42 +58,26 @@ export async function deleteFileOrDirectory(path: string): Promise<void> {
6058
export async function readDirectoryContents(path: string): Promise<FileEntry[]> {
6159
try {
6260
const entries = await platformReadDirectory(path);
63-
const workspaceRoot = useFileSystemStore.getState().rootFolderPath;
6461

6562
const filteredEntries = (entries as any[]).filter((entry: any) => {
6663
const name = entry.name || "Unknown";
6764
const isDir = entry.is_dir || false;
6865
return !shouldIgnore(name, isDir);
6966
});
7067

71-
const entriesWithSymlinkInfo = await Promise.all(
72-
filteredEntries.map(async (entry: any) => {
73-
const entryPath = entry.path || `${path}/${entry.name}`;
74-
75-
try {
76-
const symlinkInfo = await getSymlinkInfo(entryPath, workspaceRoot);
77-
78-
return {
79-
name: entry.name || "Unknown",
80-
path: entryPath,
81-
isDir: symlinkInfo.is_symlink ? false : entry.is_dir || false,
82-
children: undefined,
83-
isSymlink: symlinkInfo.is_symlink,
84-
symlinkTarget: symlinkInfo.target,
85-
};
86-
} catch (error) {
87-
console.error(`Failed to get symlink info for ${entryPath}:`, error);
88-
return {
89-
name: entry.name || "Unknown",
90-
path: entryPath,
91-
isDir: entry.is_dir || false,
92-
children: undefined,
93-
};
94-
}
95-
}),
96-
);
97-
98-
return entriesWithSymlinkInfo;
68+
// Skip per-entry symlink resolution during initial directory read.
69+
// Symlink info is resolved lazily when a file is opened (handleFileSelect)
70+
// or a directory is expanded, avoiding hundreds of stat syscalls that
71+
// cause significant lag on large projects (see #572).
72+
return filteredEntries.map((entry: any) => {
73+
const entryPath = entry.path || `${path}/${entry.name}`;
74+
return {
75+
name: entry.name || "Unknown",
76+
path: entryPath,
77+
isDir: entry.is_dir || false,
78+
children: undefined,
79+
};
80+
});
9981
} catch (error) {
10082
throw new Error(`Failed to read directory ${path}: ${error}`);
10183
}

src/features/file-system/controllers/store.ts

Lines changed: 40 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ import {
4242
createNewFile,
4343
deleteFileOrDirectory,
4444
readDirectoryContents,
45-
readFileContent,
4645
} from "./file-operations";
4746
import {
4847
addFileToTree,
@@ -791,38 +790,6 @@ export const useFileSystemStore = createSelectors(
791790
);
792791
fileOpenBenchmark.finish(path, "binary-buffer-opened");
793792
} else {
794-
if (!path.startsWith("remote://")) {
795-
try {
796-
const fileData = await readFile(resolvedPath);
797-
798-
if (isStaleRequest()) return;
799-
800-
if (isBinaryContent(fileData)) {
801-
openBuffer(
802-
path,
803-
fileName,
804-
"",
805-
false,
806-
undefined,
807-
false,
808-
false,
809-
undefined,
810-
false,
811-
false,
812-
false,
813-
undefined,
814-
false,
815-
false,
816-
true,
817-
);
818-
fileOpenBenchmark.finish(path, "binary-sniff-buffer-opened");
819-
return;
820-
}
821-
} catch (error) {
822-
console.error("Failed to inspect file bytes before opening:", error);
823-
}
824-
}
825-
826793
// Check if external editor is enabled for text files
827794
const { settings } = useSettingsStore.getState();
828795
const { openExternalEditorBuffer } = useBufferStore.getState().actions;
@@ -867,7 +834,38 @@ export const useFileSystemStore = createSelectors(
867834
filePath: remotePath,
868835
});
869836
} else {
870-
content = await readFileContent(resolvedPath);
837+
// Read file as binary first to perform binary sniffing without
838+
// a separate read pass (avoids reading large files twice, see #572).
839+
const fileData = await readFile(resolvedPath);
840+
fileOpenBenchmark.mark(path, "file-read-bytes", `${fileData.length} bytes`);
841+
842+
if (isStaleRequest()) return;
843+
844+
if (isBinaryContent(fileData)) {
845+
openBuffer(
846+
path,
847+
fileName,
848+
"",
849+
false,
850+
undefined,
851+
false,
852+
false,
853+
undefined,
854+
false,
855+
false,
856+
false,
857+
undefined,
858+
false,
859+
false,
860+
true,
861+
);
862+
fileOpenBenchmark.finish(path, "binary-sniff-buffer-opened");
863+
return;
864+
}
865+
866+
// Decode the already-read bytes instead of reading the file again
867+
const decoder = new TextDecoder("utf-8");
868+
content = decoder.decode(fileData);
871869
}
872870
fileOpenBenchmark.mark(path, "file-read", `${content.length} chars`);
873871

@@ -1914,6 +1912,14 @@ export const useFileSystemStore = createSelectors(
19141912

19151913
useWorkspaceTabsStore.getState().setActiveProjectTab(projectId);
19161914

1915+
// Close old project's buffers BEFORE loading new project to prevent
1916+
// race conditions between session save and restore, and to ensure
1917+
// terminal PTY processes from the old workspace are cleaned up
1918+
// before new ones are spawned.
1919+
if (currentBufferIds.length > 0) {
1920+
bufferActions.closeBuffersBatch(currentBufferIds, true);
1921+
}
1922+
19171923
if (remoteTabInfo) {
19181924
const reconnected = await get().handleOpenRemoteProject(
19191925
remoteTabInfo.connectionId,
@@ -2060,10 +2066,6 @@ export const useFileSystemStore = createSelectors(
20602066
})();
20612067
}
20622068

2063-
if (currentBufferIds.length > 0) {
2064-
bufferActions.closeBuffersBatch(currentBufferIds, true);
2065-
}
2066-
20672069
set((state) => {
20682070
state.isSwitchingProject = false;
20692071
});

0 commit comments

Comments
 (0)