Clarify changed and deleted files in the file viewer (#2670)
* fix(app): distinguish changed and deleted files Successful reads now reconcile transient missing events without letting stale cached data mask a deletion. File notices expose only actions valid for the current buffer state. * fix(app): preserve file state ordering * fix(app): allow retrying file checks * fix(app): clear unchanged file conflicts * fix(app): keep file conflict recovery consistent Own disk observation and read ordering in one model so stale reads and failed subscriptions cannot produce contradictory actions. * fix(app): allow retrying file preview errors Preserve daemon read errors and expose retry for initial and non-editable file loads.
This commit is contained in:
Mohamed Boudra
@@ -0,0 +1,262 @@
|
||||
import { mkdir, readFile, rename, rm, unlink, writeFile } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { expect, test, type Page } from "./fixtures";
|
||||
import { expectFileTabOpen, openFileExplorer, openFileFromExplorer } from "./helpers/file-explorer";
|
||||
import type { WithWorkspace } from "./helpers/with-workspace";
|
||||
import { installDaemonWebSocketGate } from "./helpers/daemon-websocket-gate";
|
||||
|
||||
function visibleEditor(page: Page) {
|
||||
return page.getByTestId("file-source-editor").filter({ visible: true }).locator(".cm-content");
|
||||
}
|
||||
|
||||
function filePane(page: Page) {
|
||||
return page.getByTestId("workspace-file-pane").filter({ visible: true });
|
||||
}
|
||||
|
||||
function fileCallout(page: Page) {
|
||||
return filePane(page).getByRole("alert");
|
||||
}
|
||||
|
||||
async function openFile(page: Page, filename: string): Promise<void> {
|
||||
await openFileExplorer(page);
|
||||
await openFileFromExplorer(page, filename);
|
||||
await expectFileTabOpen(page, filename);
|
||||
}
|
||||
|
||||
async function openTrackedFile(
|
||||
page: Page,
|
||||
withWorkspace: WithWorkspace,
|
||||
input: { prefix: string; relativePath: string; content: string | Uint8Array },
|
||||
): Promise<string> {
|
||||
const workspace = await withWorkspace({ prefix: input.prefix });
|
||||
const filePath = path.join(workspace.repoPath, input.relativePath);
|
||||
if (typeof input.content === "string") {
|
||||
await writeFile(filePath, input.content, "utf8");
|
||||
} else {
|
||||
await writeFile(filePath, input.content);
|
||||
}
|
||||
await workspace.navigateTo();
|
||||
await openFile(page, input.relativePath);
|
||||
return filePath;
|
||||
}
|
||||
|
||||
async function replaceEditorText(page: Page, content: string): Promise<void> {
|
||||
const editor = visibleEditor(page);
|
||||
await editor.click();
|
||||
await editor.press("Control+A");
|
||||
await editor.type(content);
|
||||
}
|
||||
|
||||
async function replaceFileAfterDeletionWasObserved(input: {
|
||||
page: Page;
|
||||
gate: Awaited<ReturnType<typeof installDaemonWebSocketGate>>;
|
||||
filePath: string;
|
||||
relativePath: string;
|
||||
content: string;
|
||||
}): Promise<void> {
|
||||
const { page, gate, filePath, relativePath, content } = input;
|
||||
// Recreate the observed race without faking file state: the daemon publishes the
|
||||
// deletion, then a real read succeeds after the replacement while its ready event waits.
|
||||
await gate.waitForFileSubscription(relativePath);
|
||||
gate.holdFileReads(relativePath);
|
||||
await unlink(filePath);
|
||||
await gate.waitForHeldFileRead();
|
||||
await expectOnlyFileCallout(page, "File deleted on disk");
|
||||
gate.holdNextReadyFileUpdate(relativePath);
|
||||
await writeFile(filePath, content, "utf8");
|
||||
gate.releaseHeldFileRead();
|
||||
await gate.waitForHeldReadyFileUpdate();
|
||||
await expect.poll(() => readFile(filePath, "utf8")).toBe(content);
|
||||
}
|
||||
|
||||
async function expectOnlyFileCallout(page: Page, title: string): Promise<void> {
|
||||
await expect(fileCallout(page)).toHaveCount(1);
|
||||
await expect(fileCallout(page)).toContainText(title);
|
||||
await expect(filePane(page).getByText(title, { exact: true })).toHaveCount(1);
|
||||
}
|
||||
|
||||
async function expectCleanReplacementCanReload(page: Page): Promise<void> {
|
||||
await expectOnlyFileCallout(page, "Changed on disk");
|
||||
await expect(filePane(page).getByText("File deleted on disk", { exact: true })).toHaveCount(0);
|
||||
await expect(fileCallout(page).getByRole("button", { name: "Overwrite" })).toHaveCount(0);
|
||||
await expect(fileCallout(page).getByRole("button", { name: "Reload" })).toBeEnabled();
|
||||
await fileCallout(page).getByRole("button", { name: "Reload" }).click();
|
||||
await expect(filePane(page).getByText("After", { exact: true })).toBeVisible();
|
||||
await expect(fileCallout(page)).toHaveCount(0);
|
||||
}
|
||||
|
||||
async function expectDeletedFileNotice(page: Page): Promise<void> {
|
||||
await expectOnlyFileCallout(page, "File deleted on disk");
|
||||
await expect(fileCallout(page)).toContainText("The open copy is preserved.");
|
||||
await expect(filePane(page).getByText("Changed on disk", { exact: true })).toHaveCount(0);
|
||||
await expect(fileCallout(page).getByRole("button", { name: "Overwrite" })).toHaveCount(0);
|
||||
await expect(fileCallout(page).getByRole("button", { name: "Reload" })).toHaveCount(0);
|
||||
}
|
||||
|
||||
async function expectDirtyConflictCanReload(page: Page): Promise<void> {
|
||||
await expectOnlyFileCallout(page, "Changed on disk");
|
||||
await expect(fileCallout(page).getByRole("button", { name: "Overwrite" })).toBeEnabled();
|
||||
await expect(fileCallout(page).getByRole("button", { name: "Reload" })).toBeEnabled();
|
||||
page.once("dialog", (dialog) => dialog.accept());
|
||||
await fileCallout(page).getByRole("button", { name: "Reload" }).click();
|
||||
await expect(visibleEditor(page)).toContainText("const external = true;");
|
||||
await expect(fileCallout(page)).toHaveCount(0);
|
||||
}
|
||||
|
||||
test.describe("Workspace file change conflicts", () => {
|
||||
test("a clean file replaced on disk offers one working reload action", async ({
|
||||
page,
|
||||
withWorkspace,
|
||||
}) => {
|
||||
const gate = await installDaemonWebSocketGate(page);
|
||||
const filePath = await openTrackedFile(page, withWorkspace, {
|
||||
prefix: "file-clean-replacement-",
|
||||
relativePath: "inventory.md",
|
||||
content: "# Before\n",
|
||||
});
|
||||
await expect(filePane(page).getByText("Before", { exact: true })).toBeVisible();
|
||||
await replaceFileAfterDeletionWasObserved({
|
||||
page,
|
||||
gate,
|
||||
filePath,
|
||||
relativePath: "inventory.md",
|
||||
content: "# After\n",
|
||||
});
|
||||
await expectCleanReplacementCanReload(page);
|
||||
gate.releaseHeldReadyFileUpdate();
|
||||
});
|
||||
|
||||
test("a deleted file shows one explanatory notice and no resolution actions", async ({
|
||||
page,
|
||||
withWorkspace,
|
||||
}) => {
|
||||
const filePath = await openTrackedFile(page, withWorkspace, {
|
||||
prefix: "file-deleted-",
|
||||
relativePath: "deleted.md",
|
||||
content: "# Present\n",
|
||||
});
|
||||
await expect(filePane(page).getByText("Present", { exact: true })).toBeVisible();
|
||||
await unlink(filePath);
|
||||
await expectDeletedFileNotice(page);
|
||||
});
|
||||
|
||||
test("a file check error offers a working retry", async ({ page, withWorkspace }) => {
|
||||
const gate = await installDaemonWebSocketGate(page);
|
||||
const relativePath = "retry.md";
|
||||
const filePath = await openTrackedFile(page, withWorkspace, {
|
||||
prefix: "file-check-retry-",
|
||||
relativePath,
|
||||
content: "# Before\n",
|
||||
});
|
||||
await gate.waitForFileSubscription(relativePath);
|
||||
await expect(filePane(page).getByText("Before", { exact: true })).toBeVisible();
|
||||
|
||||
await unlink(filePath);
|
||||
await mkdir(filePath);
|
||||
await expectOnlyFileCallout(page, "Couldn't check file on disk");
|
||||
await expect(fileCallout(page).getByRole("button", { name: "Retry" })).toBeEnabled();
|
||||
await expect(fileCallout(page).getByRole("button", { name: "Reload" })).toHaveCount(0);
|
||||
await expect(fileCallout(page).getByRole("button", { name: "Overwrite" })).toHaveCount(0);
|
||||
|
||||
gate.holdNextReadyFileUpdate(relativePath);
|
||||
await rm(filePath, { recursive: true });
|
||||
await writeFile(filePath, "# After\n", "utf8");
|
||||
await gate.waitForHeldReadyFileUpdate();
|
||||
gate.holdFileReads(relativePath);
|
||||
await fileCallout(page).getByRole("button", { name: "Retry" }).click();
|
||||
await gate.waitForHeldFileRead();
|
||||
await expect(fileCallout(page).getByRole("button", { name: "Retry" })).toBeDisabled();
|
||||
gate.releaseHeldFileRead();
|
||||
await expectCleanReplacementCanReload(page);
|
||||
gate.releaseHeldReadyFileUpdate();
|
||||
});
|
||||
|
||||
test("retry clears a transient error when the file is unchanged", async ({
|
||||
page,
|
||||
withWorkspace,
|
||||
}) => {
|
||||
const gate = await installDaemonWebSocketGate(page);
|
||||
const relativePath = "unchanged.md";
|
||||
const filePath = await openTrackedFile(page, withWorkspace, {
|
||||
prefix: "file-check-unchanged-",
|
||||
relativePath,
|
||||
content: "# Unchanged\n",
|
||||
});
|
||||
const parkedPath = `${filePath}.parked`;
|
||||
await gate.waitForFileSubscription(relativePath);
|
||||
await expect(filePane(page).getByText("Unchanged", { exact: true })).toBeVisible();
|
||||
|
||||
await rename(filePath, parkedPath);
|
||||
await mkdir(filePath);
|
||||
await expectOnlyFileCallout(page, "Couldn't check file on disk");
|
||||
|
||||
gate.holdNextReadyFileUpdate(relativePath);
|
||||
await rm(filePath, { recursive: true });
|
||||
await rename(parkedPath, filePath);
|
||||
await gate.waitForHeldReadyFileUpdate();
|
||||
await fileCallout(page).getByRole("button", { name: "Retry" }).click();
|
||||
|
||||
await expect(fileCallout(page)).toHaveCount(0);
|
||||
await expect(filePane(page).getByText("Changed on disk", { exact: true })).toHaveCount(0);
|
||||
gate.releaseHeldReadyFileUpdate();
|
||||
});
|
||||
|
||||
test("a read-only file error preserves its message and offers retry", async ({
|
||||
page,
|
||||
withWorkspace,
|
||||
}) => {
|
||||
const gate = await installDaemonWebSocketGate(page);
|
||||
const relativePath = "artifact.bin";
|
||||
const binaryContent = new Uint8Array([0, 1, 2, 3, 255]);
|
||||
const filePath = await openTrackedFile(page, withWorkspace, {
|
||||
prefix: "file-read-only-retry-",
|
||||
relativePath,
|
||||
content: binaryContent,
|
||||
});
|
||||
await gate.waitForFileSubscription(relativePath);
|
||||
await expect(
|
||||
filePane(page).getByText("Binary preview unavailable", { exact: true }),
|
||||
).toBeVisible();
|
||||
|
||||
await unlink(filePath);
|
||||
await mkdir(filePath);
|
||||
await expect(
|
||||
filePane(page).getByText("Requested path is not a file", { exact: true }),
|
||||
).toBeVisible();
|
||||
await expect(filePane(page).getByRole("button", { name: "Retry" })).toBeEnabled();
|
||||
|
||||
gate.holdNextReadyFileUpdate(relativePath);
|
||||
await rm(filePath, { recursive: true });
|
||||
await writeFile(filePath, binaryContent);
|
||||
await gate.waitForHeldReadyFileUpdate();
|
||||
gate.holdFileReads(relativePath);
|
||||
await filePane(page).getByRole("button", { name: "Retry" }).click();
|
||||
await gate.waitForHeldFileRead();
|
||||
await expect(
|
||||
filePane(page).getByText("Requested path is not a file", { exact: true }),
|
||||
).toHaveCount(0);
|
||||
gate.releaseHeldFileRead();
|
||||
|
||||
await expect(
|
||||
filePane(page).getByText("Binary preview unavailable", { exact: true }),
|
||||
).toBeVisible();
|
||||
await expect(
|
||||
filePane(page).getByText("Requested path is not a file", { exact: true }),
|
||||
).toHaveCount(0);
|
||||
gate.releaseHeldReadyFileUpdate();
|
||||
});
|
||||
|
||||
test("a changed file with local edits offers overwrite and a working reload", async ({
|
||||
page,
|
||||
withWorkspace,
|
||||
}) => {
|
||||
const filePath = await openTrackedFile(page, withWorkspace, {
|
||||
prefix: "file-dirty-conflict-",
|
||||
relativePath: "source.ts",
|
||||
content: "const before = true;\n",
|
||||
});
|
||||
await replaceEditorText(page, "const local = true;\n");
|
||||
await writeFile(filePath, "const external = true;\n", "utf8");
|
||||
await expectDirtyConflictCanReload(page);
|
||||
});
|
||||
});
|
||||
@@ -16,6 +16,21 @@ interface ClientRequest {
|
||||
type?: unknown;
|
||||
subscribe?: unknown;
|
||||
page?: { cursor?: unknown };
|
||||
mode?: unknown;
|
||||
path?: unknown;
|
||||
}
|
||||
|
||||
interface ServerMessage {
|
||||
type?: unknown;
|
||||
payload?: {
|
||||
initial?: {
|
||||
path?: unknown;
|
||||
};
|
||||
version?: {
|
||||
status?: unknown;
|
||||
path?: unknown;
|
||||
};
|
||||
};
|
||||
}
|
||||
|
||||
function readClientRequest(message: string | Buffer): ClientRequest | null {
|
||||
@@ -31,6 +46,18 @@ function readClientRequest(message: string | Buffer): ClientRequest | null {
|
||||
}
|
||||
}
|
||||
|
||||
function readServerMessage(message: string | Buffer): ServerMessage | null {
|
||||
try {
|
||||
const envelope = JSON.parse(typeof message === "string" ? message : message.toString()) as {
|
||||
type?: unknown;
|
||||
message?: ServerMessage;
|
||||
};
|
||||
return envelope.type === "session" ? (envelope.message ?? null) : envelope;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function directoryForRequest(request: ClientRequest): keyof DirectoryBootstrapCounts | null {
|
||||
if (request.page?.cursor) return null;
|
||||
if (request.type === "fetch_agents_request") return "agents";
|
||||
@@ -47,6 +74,16 @@ export async function installDaemonWebSocketGate(page: Page) {
|
||||
total: { agents: 0, workspaces: 0 },
|
||||
};
|
||||
const clientRequestCounts = new Map<string, number>();
|
||||
const subscribedFilePaths = new Set<string>();
|
||||
const fileSubscriptionWaiters = new Map<string, () => void>();
|
||||
let fileReadPathToHold: string | null = null;
|
||||
let heldFileReads: Array<() => void> = [];
|
||||
let resolveHeldFileRead: (() => void) | null = null;
|
||||
let heldFileReadPromise = Promise.resolve();
|
||||
let readyFileUpdatePathToHold: string | null = null;
|
||||
let heldReadyFileUpdate: (() => void) | null = null;
|
||||
let resolveHeldReadyFileUpdate: (() => void) | null = null;
|
||||
let heldReadyFileUpdatePromise = Promise.resolve();
|
||||
|
||||
await page.routeWebSocket(daemonWsRoutePattern(), (ws) => {
|
||||
if (!acceptingConnections) {
|
||||
@@ -69,6 +106,15 @@ export async function installDaemonWebSocketGate(page: Page) {
|
||||
directoryStarts.total[directory] += 1;
|
||||
}
|
||||
}
|
||||
if (
|
||||
request?.type === "file_explorer_request" &&
|
||||
request.mode === "file" &&
|
||||
request.path === fileReadPathToHold
|
||||
) {
|
||||
heldFileReads.push(() => server.send(message));
|
||||
resolveHeldFileRead?.();
|
||||
return;
|
||||
}
|
||||
try {
|
||||
server.send(message);
|
||||
} catch {
|
||||
@@ -78,6 +124,26 @@ export async function installDaemonWebSocketGate(page: Page) {
|
||||
|
||||
server.onMessage((message) => {
|
||||
if (!acceptingConnections) return;
|
||||
const serverMessage = readServerMessage(message);
|
||||
if (
|
||||
serverMessage?.type === "fs.file.subscribe.response" &&
|
||||
typeof serverMessage.payload?.initial?.path === "string"
|
||||
) {
|
||||
const path = serverMessage.payload.initial.path;
|
||||
subscribedFilePaths.add(path);
|
||||
fileSubscriptionWaiters.get(path)?.();
|
||||
fileSubscriptionWaiters.delete(path);
|
||||
}
|
||||
if (
|
||||
serverMessage?.type === "fs.file.update" &&
|
||||
serverMessage.payload?.version?.status === "ready" &&
|
||||
serverMessage.payload.version.path === readyFileUpdatePathToHold &&
|
||||
!heldReadyFileUpdate
|
||||
) {
|
||||
heldReadyFileUpdate = () => ws.send(message);
|
||||
resolveHeldReadyFileUpdate?.();
|
||||
return;
|
||||
}
|
||||
try {
|
||||
ws.send(message);
|
||||
} catch {
|
||||
@@ -87,6 +153,44 @@ export async function installDaemonWebSocketGate(page: Page) {
|
||||
});
|
||||
|
||||
return {
|
||||
waitForFileSubscription(path: string): Promise<void> {
|
||||
if (subscribedFilePaths.has(path)) return Promise.resolve();
|
||||
return new Promise((resolve) => fileSubscriptionWaiters.set(path, resolve));
|
||||
},
|
||||
holdFileReads(path: string): void {
|
||||
fileReadPathToHold = path;
|
||||
heldFileReads = [];
|
||||
heldFileReadPromise = new Promise((resolve) => {
|
||||
resolveHeldFileRead = resolve;
|
||||
});
|
||||
},
|
||||
waitForHeldFileRead(): Promise<void> {
|
||||
return heldFileReadPromise;
|
||||
},
|
||||
releaseHeldFileRead(): void {
|
||||
const forwards = heldFileReads;
|
||||
heldFileReads = [];
|
||||
fileReadPathToHold = null;
|
||||
resolveHeldFileRead = null;
|
||||
for (const forward of forwards) forward();
|
||||
},
|
||||
holdNextReadyFileUpdate(path: string): void {
|
||||
readyFileUpdatePathToHold = path;
|
||||
heldReadyFileUpdate = null;
|
||||
heldReadyFileUpdatePromise = new Promise((resolve) => {
|
||||
resolveHeldReadyFileUpdate = resolve;
|
||||
});
|
||||
},
|
||||
waitForHeldReadyFileUpdate(): Promise<void> {
|
||||
return heldReadyFileUpdatePromise;
|
||||
},
|
||||
releaseHeldReadyFileUpdate(): void {
|
||||
const forward = heldReadyFileUpdate;
|
||||
heldReadyFileUpdate = null;
|
||||
readyFileUpdatePathToHold = null;
|
||||
resolveHeldReadyFileUpdate = null;
|
||||
forward?.();
|
||||
},
|
||||
async drop(): Promise<void> {
|
||||
acceptingConnections = false;
|
||||
const sockets = Array.from(activeSockets);
|
||||
|
||||
Reference in New Issue
Block a user