Show an error for missing project paths (#1490)

* Fix missing project paths flashing in the sidebar * Log project lifecycle changes * Address project picker review feedback * Show project picker transport errors * Inject session filesystem for workspace tests
2026-06-12 23:42:21 +08:00
parent c4d544baf6
commit a929ea9726
20 changed files with 603 additions and 73 deletions
@@ -0,0 +1,93 @@
+import os from "node:os";
+import path from "node:path";
+import { mkdtemp, rm } from "node:fs/promises";
+import { afterEach, beforeEach, expect, test } from "vitest";
+
+import { DaemonClient, type DaemonEvent } from "../test-utils/daemon-client.js";
+import { createTestPaseoDaemon, type TestPaseoDaemon } from "../test-utils/paseo-daemon.js";
+
+const cleanupPaths = new Set<string>();
+const cleanupDaemons = new Set<TestPaseoDaemon>();
+const cleanupClients = new Set<DaemonClient>();
+let previousSupervised: string | undefined;
+
+beforeEach(() => {
+  previousSupervised = process.env.PASEO_SUPERVISED;
+  process.env.PASEO_SUPERVISED = "0";
+});
+
+afterEach(async () => {
+  restoreSupervisedEnv();
+  await Promise.all(Array.from(cleanupClients, (client) => client.close().catch(() => undefined)));
+  cleanupClients.clear();
+  await Promise.all(Array.from(cleanupDaemons, (daemon) => daemon.close().catch(() => undefined)));
+  cleanupDaemons.clear();
+  await Promise.all(
+    Array.from(cleanupPaths, (target) => rm(target, { recursive: true, force: true })),
+  );
+  cleanupPaths.clear();
+});
+
+function restoreSupervisedEnv(): void {
+  if (previousSupervised === undefined) {
+    delete process.env.PASEO_SUPERVISED;
+    return;
+  }
+
+  process.env.PASEO_SUPERVISED = previousSupervised;
+}
+
+// Repro for the "project flashes in the sidebar then disappears" report.
+// The project picker submits the typed query verbatim, so open_project can
+// receive a path that does not exist on the daemon's disk. The daemon must not
+// answer with a success + workspace upsert that it immediately retracts.
+test("openProject on a nonexistent directory does not broadcast an upsert that is immediately removed", async () => {
+  const daemon = await createTestPaseoDaemon();
+  cleanupDaemons.add(daemon);
+
+  const client = new DaemonClient({ url: `ws://127.0.0.1:${daemon.port}/ws` });
+  cleanupClients.add(client);
+  await client.connect();
+  await client.fetchAgents({ subscribe: { subscriptionId: "missing-dir-agents" } });
+
+  const workspaceEvents: Array<{ kind: "upsert" | "remove"; workspaceId: string }> = [];
+  client.subscribe((event: DaemonEvent) => {
+    if (event.type !== "workspace_update") {
+      return;
+    }
+    workspaceEvents.push({ kind: event.payload.kind, workspaceId: event.workspaceId });
+  });
+  await client.fetchWorkspaces({ subscribe: { subscriptionId: "missing-dir-workspaces" } });
+
+  const tempParent = await mkdtemp(path.join(os.tmpdir(), "paseo-open-project-"));
+  cleanupPaths.add(tempParent);
+  const missingPath = path.join(tempParent, "this-directory-does-not-exist");
+  const response = await client.openProject(missingPath);
+
+  expect(response.workspace).toBeNull();
+  expect(response.error).toBe(`Directory not found: ${missingPath}`);
+  expect(response.errorCode).toBe("directory_not_found");
+  expect(workspaceEvents).toEqual([]);
+}, 30000);
+
+test("openProject expands tilde before creating the workspace", async () => {
+  const daemon = await createTestPaseoDaemon();
+  cleanupDaemons.add(daemon);
+
+  const client = new DaemonClient({ url: `ws://127.0.0.1:${daemon.port}/ws` });
+  cleanupClients.add(client);
+  await client.connect();
+  await client.fetchAgents({ subscribe: { subscriptionId: "tilde-project-agents" } });
+  await client.fetchWorkspaces({ subscribe: { subscriptionId: "tilde-project-workspaces" } });
+
+  const home = process.env.HOME || os.homedir();
+  const workspacePath = await mkdtemp(path.join(home, ".paseo-open-project-"));
+  cleanupPaths.add(workspacePath);
+  const queryPath = `~/${path.relative(home, workspacePath)}`;
+
+  const response = await client.openProject(queryPath);
+
+  expect(response.error).toBeNull();
+  expect(response.errorCode).toBeUndefined();
+  expect(response.workspace?.workspaceDirectory).toBe(workspacePath);
+}, 30000);
@@ -2,6 +2,7 @@ import equal from "fast-deep-equal";
 import { v4 as uuidv4 } from "uuid";
 import { realpathSync } from "node:fs";
 import type { FSWatcher } from "node:fs";
+import { stat } from "node:fs/promises";
 import { basename, resolve, sep } from "path";
 import { homedir } from "node:os";
 import { z } from "zod";
@@ -533,6 +534,17 @@ interface AudioBufferState {
  totalPCMBytes: number;
 }

+export interface SessionFileSystem {
+  isDirectory(path: string): Promise<boolean>;
+}
+
+const nodeSessionFileSystem: SessionFileSystem = {
+  async isDirectory(path) {
+    const stats = await stat(path).catch(() => null);
+    return stats?.isDirectory() ?? false;
+  },
+};
+
 // Stub types for features under development (modules not yet available)
 type AgentMcpTransportFactory = () => Promise<unknown>;

@@ -564,6 +576,7 @@ export interface SessionOptions {
  agentStorage: AgentStorage;
  projectRegistry: ProjectRegistry;
  workspaceRegistry: WorkspaceRegistry;
+  filesystem?: SessionFileSystem;
  chatService: FileBackedChatService;
  scheduleService: ScheduleService;
  loopService: LoopService;
@@ -719,6 +732,19 @@ interface AgentTimelineProjectionSelection {
  hasNewer: boolean;
 }

+type RegistryTransition = "created" | "unarchived" | "existing";
+
+interface ArchivedRecordSnapshot {
+  archivedAt?: string | null;
+}
+
+function describeRegistryTransition(record: ArchivedRecordSnapshot | null): RegistryTransition {
+  if (!record) {
+    return "created";
+  }
+  return record.archivedAt ? "unarchived" : "existing";
+}
+
 /**
 * Session represents a single connected client session.
 * It owns all state management, orchestration logic, and message processing.
@@ -770,6 +796,7 @@ export class Session {
  private readonly agentStorage: AgentStorage;
  private readonly projectRegistry: ProjectRegistry;
  private readonly workspaceRegistry: WorkspaceRegistry;
+  private readonly filesystem: SessionFileSystem;
  private readonly chatService: FileBackedChatService;
  private readonly scheduleService: ScheduleService;
  private readonly loopService: LoopService;
@@ -844,6 +871,7 @@ export class Session {
      agentStorage,
      projectRegistry,
      workspaceRegistry,
+      filesystem,
      chatService,
      scheduleService,
      loopService,
@@ -893,6 +921,7 @@ export class Session {
    this.agentStorage = agentStorage;
    this.projectRegistry = projectRegistry;
    this.workspaceRegistry = workspaceRegistry;
+    this.filesystem = filesystem ?? nodeSessionFileSystem;
    this.chatService = chatService;
    this.scheduleService = scheduleService;
    this.loopService = loopService;
@@ -6755,9 +6784,10 @@ export class Session {
  }

  private async archiveWorkspaceRecord(workspaceId: string, archivedAt?: string): Promise<void> {
+    const archiveTimestamp = archivedAt ?? new Date().toISOString();
    const existingWorkspace = await archivePersistedWorkspaceRecord({
      workspaceId,
-      archivedAt,
+      archivedAt: archiveTimestamp,
      workspaceRegistry: this.workspaceRegistry,
      projectRegistry: this.projectRegistry,
    });
@@ -6766,6 +6796,22 @@ export class Session {
      return;
    }

+    if (!existingWorkspace.archivedAt) {
+      const activeSiblings = (await this.workspaceRegistry.list()).filter(
+        (workspace) => workspace.projectId === existingWorkspace.projectId && !workspace.archivedAt,
+      );
+      this.sessionLogger.info(
+        {
+          workspaceId,
+          workspaceCwd: existingWorkspace.cwd,
+          projectId: existingWorkspace.projectId,
+          projectArchived: activeSiblings.length === 0,
+          archivedAt: archiveTimestamp,
+        },
+        "Workspace archived",
+      );
+    }
+
    await this.removeWorkspaceGitWatchTarget(existingWorkspace.cwd);
    this.scriptRuntimeStore?.removeForWorkspace(existingWorkspace.cwd);
    this.removeWorkspaceGitSubscription(workspaceId);
@@ -7156,11 +7202,58 @@ export class Session {
  private async handleOpenProjectRequest(
    request: Extract<SessionInboundMessage, { type: "open_project_request" }>,
  ): Promise<void> {
+    const requestedCwd = request.cwd;
+    const cwd = expandTilde(requestedCwd);
+    const directoryExists = await this.filesystem.isDirectory(cwd).catch(() => false);
+    if (!directoryExists) {
+      this.sessionLogger.info(
+        { requestedCwd, resolvedCwd: cwd, reason: "directory_not_found" },
+        "Open project rejected",
+      );
+      this.emit({
+        type: "open_project_response",
+        payload: {
+          requestId: request.requestId,
+          workspace: null,
+          error: `Directory not found: ${cwd}`,
+          errorCode: "directory_not_found",
+        },
+      });
+      return;
+    }
+
    try {
-      const workspace = await this.findOrCreateWorkspaceForDirectory(request.cwd);
+      const projectsBefore = new Map<string, PersistedProjectRecord>();
+      for (const project of await this.projectRegistry.list()) {
+        projectsBefore.set(project.projectId, project);
+      }
+      const workspacesBefore = new Map<string, PersistedWorkspaceRecord>();
+      for (const workspaceRecord of await this.workspaceRegistry.list()) {
+        workspacesBefore.set(workspaceRecord.workspaceId, workspaceRecord);
+      }
+      const workspace = await this.findOrCreateWorkspaceForDirectory(cwd);
+      const project = await this.projectRegistry.get(workspace.projectId);
      await this.syncWorkspaceGitObserverForWorkspace(workspace);
      const descriptor = await this.describeWorkspaceRecord(workspace);
      await this.emitWorkspaceUpdateForCwd(workspace.cwd);
+      this.sessionLogger.info(
+        {
+          requestedCwd,
+          resolvedCwd: cwd,
+          workspaceCwd: workspace.cwd,
+          workspaceId: workspace.workspaceId,
+          workspaceKind: workspace.kind,
+          workspaceTransition: describeRegistryTransition(
+            workspacesBefore.get(workspace.workspaceId) ?? null,
+          ),
+          projectId: workspace.projectId,
+          projectKind: project?.kind ?? null,
+          projectTransition: describeRegistryTransition(
+            projectsBefore.get(workspace.projectId) ?? null,
+          ),
+        },
+        "Project opened",
+      );
      this.emit({
        type: "open_project_response",
        payload: {
@@ -7183,7 +7276,7 @@ export class Session {
        });
    } catch (error) {
      const message = error instanceof Error ? error.message : "Failed to open project";
-      this.sessionLogger.error({ err: error, cwd: request.cwd }, "Failed to open project");
+      this.sessionLogger.error({ err: error, cwd }, "Failed to open project");
      this.emit({
        type: "open_project_response",
        payload: {
@@ -138,6 +138,7 @@ function createHarness(input: {
        workspaces.delete(id);
      },
    }),
+    filesystem: { isDirectory: async () => true },
    chatService: createStub<SessionOptions["chatService"]>({}),
    scheduleService: createStub<SessionOptions["scheduleService"]>({}),
    loopService: createStub<SessionOptions["loopService"]>({}),
@@ -512,6 +512,7 @@ function createSessionForWorkspaceTests(
        archive: async () => {},
        remove: async () => {},
      },
+      filesystem: { isDirectory: async () => true },
      chatService: asChatService(),
      scheduleService: asScheduleService(),
      loopService: asLoopService(),
@@ -73,6 +73,26 @@ function createTestLogger() {
  return logger as unknown as pino.Logger;
 }

+interface CapturedLogRecord {
+  message: string;
+  payload: unknown;
+}
+
+function createCapturingLogger() {
+  const infoRecords: CapturedLogRecord[] = [];
+  const logger = {
+    child: () => logger,
+    trace: () => undefined,
+    debug: () => undefined,
+    info: (payload: unknown, message?: string) => {
+      infoRecords.push({ payload, message: message ?? "" });
+    },
+    warn: () => undefined,
+    error: () => undefined,
+  };
+  return { logger: logger as unknown as pino.Logger, infoRecords };
+}
+
 function createWorkspaceGitServiceStub(
  metadataByCwd: Record<
    string,
@@ -692,4 +712,80 @@ describe("WorkspaceReconciliationService", () => {
    expect(onChanges).toHaveBeenCalledTimes(1);
    expect(onChanges.mock.calls[0][0].length).toBeGreaterThan(0);
  });
+
+  test("logs reconciliation changes with affected paths and reasons", async () => {
+    const { projects, workspaces, projectRegistry, workspaceRegistry } = createTestRegistries();
+    const { logger, infoRecords } = createCapturingLogger();
+
+    projects.set(
+      "p1",
+      createPersistedProjectRecord({
+        projectId: "p1",
+        rootPath: "/tmp/does-not-exist-log-test",
+        kind: "non_git",
+        displayName: "ghost",
+        createdAt: timestamp,
+        updatedAt: timestamp,
+      }),
+    );
+    workspaces.set(
+      "w1",
+      createPersistedWorkspaceRecord({
+        workspaceId: "w1",
+        projectId: "p1",
+        cwd: "/tmp/does-not-exist-log-test",
+        kind: "directory",
+        displayName: "ghost",
+        createdAt: timestamp,
+        updatedAt: timestamp,
+      }),
+    );
+
+    const service = new WorkspaceReconciliationService({
+      projectRegistry,
+      workspaceRegistry,
+      logger,
+    });
+
+    await service.runOnce();
+
+    expect(infoRecords).toEqual([
+      {
+        message: "Workspace reconciliation applied changes",
+        payload: expect.objectContaining({
+          changeCount: 2,
+          changes: expect.arrayContaining([
+            {
+              kind: "workspace_archived",
+              workspaceId: "w1",
+              directory: "/tmp/does-not-exist-log-test",
+              reason: "directory_missing",
+            },
+            {
+              kind: "project_archived",
+              projectId: "p1",
+              directory: "/tmp/does-not-exist-log-test",
+              reason: "no_active_workspaces",
+            },
+          ]),
+          durationMs: expect.any(Number),
+        }),
+      },
+    ]);
+  });
+
+  test("does not log reconciliation when no changes are applied", async () => {
+    const { projectRegistry, workspaceRegistry } = createTestRegistries();
+    const { logger, infoRecords } = createCapturingLogger();
+
+    const service = new WorkspaceReconciliationService({
+      projectRegistry,
+      workspaceRegistry,
+      logger,
+    });
+
+    await service.runOnce();
+
+    expect(infoRecords).toEqual([]);
+  });
 });
@@ -109,13 +109,7 @@ export class WorkspaceReconciliationService {
    if (this.running) return;
    this.running = true;
    try {
-      const result = await this.reconcile();
-      if (result.changesApplied.length > 0) {
-        this.logger.info(
-          { changeCount: result.changesApplied.length, durationMs: result.durationMs },
-          "Reconciliation pass completed with changes",
-        );
-      }
+      await this.reconcile();
    } catch (error) {
      this.logger.error({ err: error }, "Reconciliation pass failed");
    } finally {
@@ -201,7 +195,19 @@ export class WorkspaceReconciliationService {
      this.onChanges(changes);
    }

-    return { changesApplied: changes, durationMs: Date.now() - start };
+    const result = { changesApplied: changes, durationMs: Date.now() - start };
+    if (changes.length > 0) {
+      this.logger.info(
+        {
+          changeCount: changes.length,
+          durationMs: result.durationMs,
+          changes,
+        },
+        "Workspace reconciliation applied changes",
+      );
+    }
+
+    return result;
  }

  private async mergeDuplicateProjectsByRoot(