From a6b89bfdf7f8ca878a4243294041c7c4d2a551a1 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 26 Apr 2026 13:39:08 +0700 Subject: [PATCH] perf: unblock open_project from gh pr status fetch Placement only ever needed local git data; fetching GitHub PR status was incidental coupling through getSnapshot, blocking the response ~5s with no feedback in the desktop app. Split it: getCheckout for placement (local only), getSnapshot for the watch path. classify is now a pure function over a checkout. The observer emits the full gitRuntime/githubRuntime via workspace_update when the background snapshot lands. --- packages/server/src/server/session.ts | 37 ++-- .../src/server/session.workspaces.test.ts | 201 ++++++++++++++++++ .../test-utils/workspace-git-service-stub.ts | 9 + .../server/src/server/websocket-server.ts | 9 + .../src/server/workspace-git-service.ts | 40 +++- .../server/workspace-registry-bootstrap.ts | 9 +- .../server/workspace-registry-model.test.ts | 38 +--- .../src/server/workspace-registry-model.ts | 36 +--- 8 files changed, 306 insertions(+), 73 deletions(-) diff --git a/packages/server/src/server/session.ts b/packages/server/src/server/session.ts index 9823e9147..539b820b6 100644 --- a/packages/server/src/server/session.ts +++ b/packages/server/src/server/session.ts @@ -1596,12 +1596,8 @@ export class Session { return normalizePersistedWorkspaceId(snapshot?.git.repoRoot ?? normalizedCwd); } - try { - const snapshot = await this.workspaceGitService.getSnapshot(normalizedCwd); - return normalizePersistedWorkspaceId(snapshot.git.repoRoot ?? normalizedCwd); - } catch { - return normalizedCwd; - } + const checkout = await this.workspaceGitService.getCheckout(normalizedCwd); + return normalizePersistedWorkspaceId(checkout.worktreeRoot ?? normalizedCwd); } private async buildProjectPlacementForWorkspace( @@ -6577,10 +6573,8 @@ export class Session { } private async createWorkspaceForDirectory(cwd: string): Promise { - const membership = await classifyDirectoryForProjectMembership({ - cwd, - workspaceGitService: this.workspaceGitService, - }); + const checkout = await this.workspaceGitService.getCheckout(cwd); + const membership = classifyDirectoryForProjectMembership({ cwd, checkout }); const timestamp = new Date().toISOString(); const projectRecord = await this.resolveProjectRecordForPlacement({ @@ -6607,10 +6601,8 @@ export class Session { project: PersistedProjectRecord | null; cwd: string; }): Promise { - const membership = await classifyDirectoryForProjectMembership({ - cwd: input.cwd, - workspaceGitService: this.workspaceGitService, - }); + const checkout = await this.workspaceGitService.getCheckout(input.cwd); + const membership = classifyDirectoryForProjectMembership({ cwd: input.cwd, checkout }); const timestamp = new Date().toISOString(); const projectRecord = await this.resolveProjectRecordForPlacement({ membership, @@ -6646,7 +6638,7 @@ export class Session { } private async resolveProjectRecordForPlacement(input: { - membership: Awaited>; + membership: ReturnType; timestamp: string; }): Promise { const rootPath = input.membership.projectRootPath; @@ -7063,8 +7055,9 @@ export class Session { ): Promise { try { const workspace = await this.findOrCreateWorkspaceForDirectory(request.cwd); + await this.syncWorkspaceGitObserverForWorkspace(workspace); + const descriptor = await this.describeWorkspaceRecord(workspace); await this.emitWorkspaceUpdateForCwd(workspace.cwd); - const descriptor = await this.describeWorkspaceRecordWithGitData(workspace); this.emit({ type: "open_project_response", payload: { @@ -7073,6 +7066,18 @@ export class Session { error: null, }, }); + void this.workspaceGitService + .getSnapshot(workspace.cwd, { + force: true, + includeGitHub: true, + reason: "open_project", + }) + .catch((error) => { + this.sessionLogger.warn( + { err: error, cwd: workspace.cwd }, + "Background snapshot refresh failed after open_project", + ); + }); } 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"); diff --git a/packages/server/src/server/session.workspaces.test.ts b/packages/server/src/server/session.workspaces.test.ts index af2a5bc7f..f3235aaa0 100644 --- a/packages/server/src/server/session.workspaces.test.ts +++ b/packages/server/src/server/session.workspaces.test.ts @@ -65,6 +65,12 @@ interface SessionTestAccess { terminalManager: { killTerminal(id: string): unknown; } | null; + workspaceGitService: { + getCheckout: (cwd: string) => Promise; + getSnapshot: (cwd: string, options?: unknown) => Promise; + peekSnapshot: (cwd: string) => WorkspaceGitRuntimeSnapshot | null; + registerWorkspace: (params: { cwd: string }, listener: unknown) => { unsubscribe: () => void }; + }; } interface ListFetchResult { @@ -2094,6 +2100,174 @@ test("open_project_request registers a workspace before any agent exists", async expect(response?.payload.workspace?.id).toBe("/tmp/repo"); }); +test("open_project_response returns immediately even when the GitHub fetch is slow", async () => { + const emitted: Array<{ type: string; payload: unknown }> = []; + const session = createSessionForWorkspaceTests(); + const projects = new Map>(); + const workspaces = new Map>(); + const cwd = "/tmp/slow-github-repo"; + + session.emit = (message) => emitted.push(message as { type: string; payload: unknown }); + session.projectRegistry.get = async (projectId: string) => projects.get(projectId) ?? null; + session.projectRegistry.upsert = async ( + record: ReturnType, + ) => { + projects.set(record.projectId, record); + }; + session.workspaceRegistry.get = async (workspaceId: string) => + workspaces.get(workspaceId) ?? null; + session.workspaceRegistry.upsert = async ( + record: ReturnType, + ) => { + workspaces.set(record.workspaceId, record); + }; + session.projectRegistry.list = async () => Array.from(projects.values()); + session.workspaceRegistry.list = async () => Array.from(workspaces.values()); + session.workspaceGitService.getCheckout = async (requestedCwd: string) => ({ + cwd: requestedCwd, + isGit: true, + currentBranch: "main", + remoteUrl: "https://github.com/acme/slow.git", + worktreeRoot: requestedCwd, + isPaseoOwnedWorktree: false, + mainRepoRoot: null, + }); + let resolveSnapshot: (snapshot: WorkspaceGitRuntimeSnapshot) => void = () => {}; + const snapshotPromise = new Promise((resolve) => { + resolveSnapshot = resolve; + }); + session.workspaceGitService.getSnapshot = (requestedCwd: string) => { + void requestedCwd; + return snapshotPromise; + }; + + const start = Date.now(); + await session.handleMessage({ + type: "open_project_request", + cwd, + requestId: "req-slow-github", + }); + const elapsedMs = Date.now() - start; + + expect(elapsedMs).toBeLessThan(500); + + const response = emitted.find((message) => message.type === "open_project_response") as + | { + payload: { + error: unknown; + workspace?: { + id: string; + gitRuntime?: unknown; + githubRuntime?: unknown; + }; + }; + } + | undefined; + expect(response?.payload.error).toBeNull(); + expect(response?.payload.workspace?.id).toBe(cwd); + expect(response?.payload.workspace?.gitRuntime).toBeUndefined(); + expect(response?.payload.workspace?.githubRuntime).toBeUndefined(); + + resolveSnapshot(createWorkspaceRuntimeSnapshot(cwd)); +}); + +test("open_project_request emits a workspace_update with githubRuntime once the snapshot resolves", async () => { + const emitted: Array<{ type: string; payload: unknown }> = []; + const session = createSessionForWorkspaceTests(); + const projects = new Map>(); + const workspaces = new Map>(); + const cwd = "/tmp/github-runtime-repo"; + const snapshot = createWorkspaceRuntimeSnapshot(cwd); + + let listener: ((snapshot: WorkspaceGitRuntimeSnapshot) => void) | null = null; + const peeked = { value: null as WorkspaceGitRuntimeSnapshot | null }; + + session.emit = (message) => emitted.push(message as { type: string; payload: unknown }); + session.projectRegistry.get = async (projectId: string) => projects.get(projectId) ?? null; + session.projectRegistry.upsert = async ( + record: ReturnType, + ) => { + projects.set(record.projectId, record); + }; + session.workspaceRegistry.get = async (workspaceId: string) => + workspaces.get(workspaceId) ?? null; + session.workspaceRegistry.upsert = async ( + record: ReturnType, + ) => { + workspaces.set(record.workspaceId, record); + }; + session.projectRegistry.list = async () => Array.from(projects.values()); + session.workspaceRegistry.list = async () => Array.from(workspaces.values()); + session.workspaceGitService.getCheckout = async (requestedCwd: string) => ({ + cwd: requestedCwd, + isGit: true, + currentBranch: "main", + remoteUrl: "https://github.com/acme/repo.git", + worktreeRoot: requestedCwd, + isPaseoOwnedWorktree: false, + mainRepoRoot: null, + }); + session.workspaceGitService.peekSnapshot = () => peeked.value; + session.workspaceGitService.registerWorkspace = (_params, incomingListener) => { + listener = incomingListener as (snapshot: WorkspaceGitRuntimeSnapshot) => void; + return { unsubscribe: () => {} }; + }; + session.workspaceGitService.getSnapshot = async () => { + peeked.value = snapshot; + listener?.(snapshot); + return snapshot; + }; + session.workspaceUpdatesSubscription = { + subscriptionId: "sub-open-project", + filter: undefined, + isBootstrapping: false, + pendingUpdatesByWorkspaceId: new Map(), + lastEmittedByWorkspaceId: new Map(), + }; + session.reconcileActiveWorkspaceRecords = async () => new Set(); + + await session.handleMessage({ + type: "open_project_request", + cwd, + requestId: "req-runtime-update", + }); + + await new Promise((resolve) => setImmediate(resolve)); + await new Promise((resolve) => setImmediate(resolve)); + + const updates = emitted.filter((message) => message.type === "workspace_update") as Array<{ + type: "workspace_update"; + payload: WorkspaceUpdatePayloadShape; + }>; + const upsertedWithGitHub = updates + .map((update) => update.payload) + .filter( + (payload): payload is WorkspaceUpsertPayload => + payload.kind === "upsert" && payload.workspace.id === cwd, + ) + .find((payload) => payload.workspace.githubRuntime?.pullRequest); + expect(upsertedWithGitHub?.workspace.githubRuntime?.pullRequest).toEqual( + expect.objectContaining({ url: "https://github.com/acme/repo/pull/123" }), + ); +}); + +interface WorkspaceUpsertPayload { + kind: "upsert"; + workspace: { + id: string; + githubRuntime?: { + pullRequest?: { url?: string } | null; + } | null; + }; +} + +interface WorkspaceRemovePayload { + kind: "remove"; + id: string; +} + +type WorkspaceUpdatePayloadShape = WorkspaceUpsertPayload | WorkspaceRemovePayload; + test("open_project_request does not match a new child directory to an existing parent workspace", async () => { const emitted: Array<{ type: string; payload: unknown }> = []; const session = createSessionForWorkspaceTests(); @@ -2278,6 +2452,15 @@ test("open_project_request reclassifies an archived directory workspace when git }; session.projectRegistry.list = async () => Array.from(projects.values()); session.workspaceRegistry.list = async () => Array.from(workspaces.values()); + session.workspaceGitService.getCheckout = async () => ({ + cwd, + isGit: true, + currentBranch: "feature/desktop-daemon-settings", + remoteUrl: "git@github.com:getpaseo/paseo.git", + worktreeRoot: cwd, + isPaseoOwnedWorktree: false, + mainRepoRoot: repoRoot, + }); session.workspaceGitService.getSnapshot = async () => createWorkspaceRuntimeSnapshot(cwd, { git: { @@ -2389,6 +2572,15 @@ test("open_project_request reclassifies an active directory workspace when git m }; session.projectRegistry.list = async () => Array.from(projects.values()); session.workspaceRegistry.list = async () => Array.from(workspaces.values()); + session.workspaceGitService.getCheckout = async (requestedCwd: string) => ({ + cwd: requestedCwd, + isGit: true, + currentBranch: requestedCwd === repoRoot ? "main" : "feature/desktop-daemon-settings", + remoteUrl: "git@github.com:getpaseo/paseo.git", + worktreeRoot: requestedCwd, + isPaseoOwnedWorktree: false, + mainRepoRoot: requestedCwd === repoRoot ? null : repoRoot, + }); session.workspaceGitService.getSnapshot = async (requestedCwd: string) => createWorkspaceRuntimeSnapshot(requestedCwd, { git: { @@ -2476,6 +2668,15 @@ test("open_project_request groups a plain git worktree under an existing repo pr }; session.projectRegistry.list = async () => Array.from(projects.values()); session.workspaceRegistry.list = async () => Array.from(workspaces.values()); + session.workspaceGitService.getCheckout = async (requestedCwd: string) => ({ + cwd: requestedCwd, + isGit: true, + currentBranch: requestedCwd === repoRoot ? "main" : "feature/desktop-daemon-settings", + remoteUrl: "git@github.com:getpaseo/paseo.git", + worktreeRoot: requestedCwd, + isPaseoOwnedWorktree: false, + mainRepoRoot: requestedCwd === repoRoot ? null : repoRoot, + }); session.workspaceGitService.getSnapshot = async (requestedCwd: string) => createWorkspaceRuntimeSnapshot(requestedCwd, { git: { diff --git a/packages/server/src/server/test-utils/workspace-git-service-stub.ts b/packages/server/src/server/test-utils/workspace-git-service-stub.ts index ef7dd5972..0d4d2ac37 100644 --- a/packages/server/src/server/test-utils/workspace-git-service-stub.ts +++ b/packages/server/src/server/test-utils/workspace-git-service-stub.ts @@ -40,6 +40,15 @@ export function createNoopWorkspaceGitService( unsubscribe: () => {}, }), peekSnapshot: () => null, + getCheckout: async (cwd: string) => ({ + cwd, + isGit: false, + currentBranch: null, + remoteUrl: null, + worktreeRoot: null, + isPaseoOwnedWorktree: false, + mainRepoRoot: null, + }), getSnapshot: async (cwd: string) => createNoGitWorkspaceRuntimeSnapshot(cwd), getCheckoutDiff: async (): Promise => ({ diff: "" }), validateBranchRef: async () => ({ kind: "not-found" }), diff --git a/packages/server/src/server/websocket-server.ts b/packages/server/src/server/websocket-server.ts index 11f8485c1..894d35c91 100644 --- a/packages/server/src/server/websocket-server.ts +++ b/packages/server/src/server/websocket-server.ts @@ -102,6 +102,15 @@ function createFallbackWorkspaceGitService(): WorkspaceGitService { unsubscribe: () => {}, }), peekSnapshot: () => null, + getCheckout: async (cwd: string) => ({ + cwd, + isGit: false, + currentBranch: null, + remoteUrl: null, + worktreeRoot: null, + isPaseoOwnedWorktree: false, + mainRepoRoot: null, + }), getSnapshot: async (cwd: string) => createFallbackWorkspaceGitSnapshot(cwd), getCheckoutDiff: async () => ({ diff: "" }), validateBranchRef: async () => ({ kind: "not-found" }), diff --git a/packages/server/src/server/workspace-git-service.ts b/packages/server/src/server/workspace-git-service.ts index dd4ed960f..a9692cf7c 100644 --- a/packages/server/src/server/workspace-git-service.ts +++ b/packages/server/src/server/workspace-git-service.ts @@ -3,6 +3,7 @@ import { readFile, readdir } from "node:fs/promises"; import { join, resolve } from "node:path"; import pLimit from "p-limit"; import type pino from "pino"; +import type { ProjectCheckoutLitePayload } from "../shared/messages.js"; import type { CheckoutContext } from "../utils/checkout-git.js"; import { type BranchCheckoutResolution, @@ -28,7 +29,7 @@ import { buildWorkspaceGitMetadataFromSnapshot, type WorkspaceGitMetadata, } from "./workspace-git-metadata.js"; -import { normalizeWorkspaceId } from "./workspace-registry-model.js"; +import { checkoutLiteFromGitSnapshot, normalizeWorkspaceId } from "./workspace-registry-model.js"; const WORKSPACE_GIT_WATCH_DEBOUNCE_MS = 500; const BACKGROUND_GIT_FETCH_INTERVAL_MS = 180_000; @@ -94,6 +95,7 @@ export interface WorkspaceGitService { ): WorkspaceGitSubscription; peekSnapshot(cwd: string): WorkspaceGitRuntimeSnapshot | null; + getCheckout(cwd: string): Promise; getSnapshot( cwd: string, options?: WorkspaceGitSnapshotOptions, @@ -390,6 +392,42 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { return this.requestWorkspaceSnapshot(target, request); } + async getCheckout(cwd: string): Promise { + const normalizedCwd = normalizeWorkspaceId(cwd); + try { + const status = await this.deps.getCheckoutStatus(normalizedCwd, { + paseoHome: this.paseoHome, + }); + if (!status.isGit) { + return checkoutLiteFromGitSnapshot(normalizedCwd, { + isGit: false, + currentBranch: null, + remoteUrl: null, + repoRoot: null, + isPaseoOwnedWorktree: false, + mainRepoRoot: null, + }); + } + return checkoutLiteFromGitSnapshot(normalizedCwd, { + isGit: true, + currentBranch: status.currentBranch, + remoteUrl: status.remoteUrl, + repoRoot: status.repoRoot, + isPaseoOwnedWorktree: status.isPaseoOwnedWorktree, + mainRepoRoot: status.mainRepoRoot, + }); + } catch { + return checkoutLiteFromGitSnapshot(normalizedCwd, { + isGit: false, + currentBranch: null, + remoteUrl: null, + repoRoot: null, + isPaseoOwnedWorktree: false, + mainRepoRoot: null, + }); + } + } + peekSnapshot(cwd: string): WorkspaceGitRuntimeSnapshot | null { cwd = normalizeWorkspaceId(cwd); return this.workspaceTargets.get(cwd)?.latestSnapshot ?? null; diff --git a/packages/server/src/server/workspace-registry-bootstrap.ts b/packages/server/src/server/workspace-registry-bootstrap.ts index 4029c749d..d75316d26 100644 --- a/packages/server/src/server/workspace-registry-bootstrap.ts +++ b/packages/server/src/server/workspace-registry-bootstrap.ts @@ -68,16 +68,17 @@ export async function bootstrapWorkspaceRegistries(options: { const recordsByWorkspaceId = new Map< string, { - membership: Awaited>; + membership: ReturnType; records: StoredAgentRecord[]; } >(); const placements = await Promise.all( activeRecords.map(async (record) => { const normalizedCwd = normalizeWorkspaceId(record.cwd); - const membership = await classifyDirectoryForProjectMembership({ + const checkout = await options.workspaceGitService.getCheckout(normalizedCwd); + const membership = classifyDirectoryForProjectMembership({ cwd: normalizedCwd, - workspaceGitService: options.workspaceGitService, + checkout, }); return { record, membership, workspaceId: membership.workspaceId }; }), @@ -91,7 +92,7 @@ export async function bootstrapWorkspaceRegistries(options: { const projectRanges = new Map(); const workspaceUpsertInputs: { workspaceId: string; - membership: Awaited>; + membership: ReturnType; workspaceCwd: string; createdAt: string; updatedAt: string; diff --git a/packages/server/src/server/workspace-registry-model.test.ts b/packages/server/src/server/workspace-registry-model.test.ts index 2f8536b1e..c16252072 100644 --- a/packages/server/src/server/workspace-registry-model.test.ts +++ b/packages/server/src/server/workspace-registry-model.test.ts @@ -109,34 +109,18 @@ describe("deriveWorkspaceId", () => { }); describe("git worktree grouping", () => { - test("classifies plain git worktrees for project membership from git facts", async () => { - const membership = await classifyDirectoryForProjectMembership({ + test("classifies plain git worktrees for project membership from git facts", () => { + const membership = classifyDirectoryForProjectMembership({ cwd: "/tmp/repo-feature", - workspaceGitService: { - getSnapshot: async () => ({ - cwd: "/tmp/repo-feature", - git: { - isGit: true, - repoRoot: "/tmp/repo-feature", - mainRepoRoot: "/tmp/repo", - currentBranch: "feature/plain", - remoteUrl: "https://github.com/acme/repo.git", - isPaseoOwnedWorktree: false, - isDirty: false, - baseRef: null, - aheadBehind: null, - aheadOfOrigin: null, - behindOfOrigin: null, - hasRemote: true, - diffStat: null, - }, - github: { - featuresEnabled: false, - pullRequest: null, - error: null, - }, - }), - } as never, + checkout: { + cwd: "/tmp/repo-feature", + isGit: true, + currentBranch: "feature/plain", + remoteUrl: "https://github.com/acme/repo.git", + worktreeRoot: "/tmp/repo-feature", + isPaseoOwnedWorktree: false, + mainRepoRoot: "/tmp/repo", + }, }); expect(membership).toMatchObject({ diff --git a/packages/server/src/server/workspace-registry-model.ts b/packages/server/src/server/workspace-registry-model.ts index 25a51a23b..051ffb979 100644 --- a/packages/server/src/server/workspace-registry-model.ts +++ b/packages/server/src/server/workspace-registry-model.ts @@ -2,7 +2,6 @@ import { resolve } from "node:path"; import type { ProjectCheckoutLitePayload, ProjectPlacementPayload } from "../shared/messages.js"; import { parseGitRevParsePath } from "../utils/git-rev-parse-path.js"; -import type { WorkspaceGitService } from "./workspace-git-service.js"; import type { PersistedWorkspaceRecord } from "./workspace-registry.js"; export type PersistedProjectKind = "git" | "non_git"; @@ -215,11 +214,11 @@ export async function detectStaleWorkspaces( return staleWorkspaceIds; } -export async function buildProjectPlacementForCwd(input: { +export function buildProjectPlacementForCwd(input: { cwd: string; - workspaceGitService: WorkspaceGitService; -}): Promise { - const membership = await classifyDirectoryForProjectMembership(input); + checkout: ProjectCheckoutLitePayload; +}): ProjectPlacementPayload { + const membership = classifyDirectoryForProjectMembership(input); return { projectKey: membership.projectKey, projectName: membership.projectName, @@ -227,28 +226,15 @@ export async function buildProjectPlacementForCwd(input: { }; } -export async function classifyDirectoryForProjectMembership(input: { +export function classifyDirectoryForProjectMembership(input: { cwd: string; - workspaceGitService: WorkspaceGitService; -}): Promise { + checkout: ProjectCheckoutLitePayload; +}): DirectoryProjectMembership { const normalizedCwd = normalizeWorkspaceId(input.cwd); - const checkout = await input.workspaceGitService - .getSnapshot(normalizedCwd) - .then( - (snapshot): ProjectCheckoutLitePayload => - checkoutLiteFromGitSnapshot(normalizedCwd, snapshot.git), - ) - .catch( - (): ProjectCheckoutLitePayload => ({ - cwd: normalizedCwd, - isGit: false, - currentBranch: null, - remoteUrl: null, - worktreeRoot: null, - isPaseoOwnedWorktree: false, - mainRepoRoot: null, - }), - ); + const checkout: ProjectCheckoutLitePayload = { + ...input.checkout, + cwd: normalizedCwd, + }; const projectKey = deriveProjectGroupingKey({ cwd: checkout.worktreeRoot ?? normalizedCwd,