diff --git a/packages/server/src/server/session.ts b/packages/server/src/server/session.ts index 06f698e76..82f93c86b 100644 --- a/packages/server/src/server/session.ts +++ b/packages/server/src/server/session.ts @@ -4081,21 +4081,12 @@ export class Session { ); } - await this.teardownArchivedWorkspace({ - workspaceId: existingWorkspace.workspaceId, - cwd: existingWorkspace.cwd, - }); + await this.teardownArchivedWorkspace(existingWorkspace.workspaceId); } - // Git watch and subscription state is keyed by directory; the script runtime - // store is keyed by the opaque workspace id. Each cleanup uses its own key so an - // opaque id is never resolved as a filesystem path. - private async teardownArchivedWorkspace(input: { - workspaceId: string; - cwd: string; - }): Promise { - this.workspaceGitObserver.removeForCwd(input.cwd); - this.scriptRuntimeStore?.removeForWorkspace(input.workspaceId); + private async teardownArchivedWorkspace(workspaceId: string): Promise { + this.workspaceGitObserver.removeForWorkspaceId(workspaceId); + this.scriptRuntimeStore?.removeForWorkspace(workspaceId); } private async reconcileAndEmitWorkspaceUpdates(): Promise { @@ -4130,10 +4121,7 @@ export class Session { result.changesApplied.map(async (change) => { switch (change.kind) { case "workspace_archived": - await this.teardownArchivedWorkspace({ - workspaceId: change.workspaceId, - cwd: change.directory, - }); + await this.teardownArchivedWorkspace(change.workspaceId); changedWorkspaceIds.add(change.workspaceId); break; case "workspace_updated": diff --git a/packages/server/src/server/session/workspace-git-observer/workspace-git-observer-service.test.ts b/packages/server/src/server/session/workspace-git-observer/workspace-git-observer-service.test.ts index 8319cca64..b86131401 100644 --- a/packages/server/src/server/session/workspace-git-observer/workspace-git-observer-service.test.ts +++ b/packages/server/src/server/session/workspace-git-observer/workspace-git-observer-service.test.ts @@ -163,6 +163,15 @@ describe("syncObservers", () => { expect(h.registerCalls).toEqual([WS1]); }); + test("shares one cwd subscription across distinct workspace identities", () => { + const h = buildHarness(); + h.service.syncObservers([ + makeDescriptor({ id: "ws1", workspaceDirectory: WS1 }), + makeDescriptor({ id: "ws2", workspaceDirectory: WS1 }), + ]); + expect(h.registerCalls).toEqual([WS1]); + }); + test("tears down the subscription when a git workspace becomes non-git", () => { const h = buildHarness(); h.service.syncObservers([makeDescriptor({ id: "ws1", workspaceDirectory: WS1 })]); @@ -176,6 +185,24 @@ describe("syncObservers", () => { ]); expect(h.unsubscribeCalls).toEqual([WS1]); }); + + test("keeps a shared subscription when one sibling becomes non-git", () => { + const h = buildHarness(); + h.service.syncObservers([ + makeDescriptor({ id: "ws1", workspaceDirectory: WS1 }), + makeDescriptor({ id: "ws2", workspaceDirectory: WS1 }), + ]); + h.service.syncObservers([ + makeDescriptor({ + id: "ws1", + workspaceDirectory: WS1, + workspaceKind: "directory", + }), + ]); + expect(h.unsubscribeCalls).toEqual([]); + h.emitSnapshot(WS1, "feature"); + expect(h.branchChanges).toEqual([["ws2", null, "feature"]]); + }); }); describe("git snapshot listener", () => { @@ -222,6 +249,13 @@ describe("shouldSkipUpdate", () => { expect(h.service.shouldSkipUpdate("ws1", a)).toBe(true); expect(h.service.shouldSkipUpdate("ws1", b)).toBe(false); }); + + test("starts from the descriptor state recorded during observer sync", () => { + const h = buildHarness(); + const descriptor = makeDescriptor({ id: "ws1", workspaceDirectory: WS1, name: "main" }); + h.service.syncObservers([descriptor]); + expect(h.service.shouldSkipUpdate("ws1", descriptor)).toBe(true); + }); }); describe("recordDescriptorState", () => { @@ -259,10 +293,25 @@ describe("teardown", () => { expect(h.unsubscribeCalls).toEqual([]); }); - test("removeForCwd unsubscribes and stops the observer", () => { + test("keeps a shared cwd subscription until its last workspace is removed", () => { const h = buildHarness(); - h.service.syncObservers([makeDescriptor({ id: "ws1", workspaceDirectory: WS1 })]); - h.service.removeForCwd(WS1); + h.service.syncObservers([ + makeDescriptor({ id: "ws1", workspaceDirectory: WS1, name: "main" }), + makeDescriptor({ id: "ws2", workspaceDirectory: WS1, name: "main" }), + ]); + + h.emitSnapshot(WS1, "feature"); + h.service.removeForWorkspaceId("ws1"); + expect(h.unsubscribeCalls).toEqual([]); + + h.emitSnapshot(WS1, "next"); + expect(h.branchChanges).toEqual([ + ["ws1", "main", "feature"], + ["ws2", "main", "feature"], + ["ws2", "feature", "next"], + ]); + + h.service.removeForWorkspaceId("ws2"); expect(h.unsubscribeCalls).toEqual([WS1]); expect(() => h.emitSnapshot(WS1, "x")).toThrow(); }); diff --git a/packages/server/src/server/session/workspace-git-observer/workspace-git-observer-service.ts b/packages/server/src/server/session/workspace-git-observer/workspace-git-observer-service.ts index 31239106a..9a26f8bc4 100644 --- a/packages/server/src/server/session/workspace-git-observer/workspace-git-observer-service.ts +++ b/packages/server/src/server/session/workspace-git-observer/workspace-git-observer-service.ts @@ -10,8 +10,11 @@ import type { PersistedWorkspaceRecord } from "../../workspace-registry.js"; const WORKSPACE_GIT_WATCH_REMOVED_STATE_KEY = "__removed__"; interface WorkspaceGitWatchTarget { + workspaceIds: Set; +} + +interface WorkspaceGitWatchState { cwd: string; - workspaceId: string; latestDescriptorStateKey: string | null; lastBranchName: string | null; } @@ -20,8 +23,9 @@ interface WorkspaceGitWatchTarget { * Observes a workspace's git state on disk (via WorkspaceGitService) and drives the * live update fan-out: branch-change notifications, workspace-card refreshes, and * checkout status updates. It owns the per-cwd watch targets and the WorkspaceGitService - * subscription handles, so the registration / dedupe / teardown lifecycle lives in one - * module instead of being smeared across the client session. + * subscription handles. Filesystem subscriptions are keyed by cwd while descriptor and + * branch state remain keyed by workspace id, so same-directory workspace records share one + * watch without sharing identity or teardown lifetime. * * Branch changes reach `onBranchChanged` from two paths that share `lastBranchName`: the * on-disk snapshot listener (handleBranchSnapshot) and the workspace-emit loop @@ -37,7 +41,6 @@ export interface WorkspaceGitObserverService { recordDescriptorState(workspaceId: string, workspace: WorkspaceDescriptorPayload | null): void; handleBranchSnapshot(cwd: string, branchName: string | null): void; removeForWorkspaceId(workspaceId: string): void; - removeForCwd(cwd: string): void; dispose(): void; } @@ -67,6 +70,7 @@ export function createWorkspaceGitObserverService(deps: { } = deps; const watchTargets = new Map(); + const workspaceStates = new Map(); const subscriptions = new Map void>(); function descriptorStateKey(workspace: WorkspaceDescriptorPayload | null): string { @@ -79,71 +83,93 @@ export function createWorkspaceGitObserverService(deps: { ]); } - function resolveTargetByWorkspaceId(workspaceId: string): WorkspaceGitWatchTarget | null { - for (const target of watchTargets.values()) { - if (target.workspaceId === workspaceId) { - return target; - } - } - return null; - } - function rememberDescriptorState( workspaceId: string, workspace: WorkspaceDescriptorPayload | null, ): void { - const target = resolveTargetByWorkspaceId(workspaceId); - if (!target) { + const state = workspaceStates.get(workspaceId); + if (!state) { return; } - target.latestDescriptorStateKey = descriptorStateKey(workspace); - target.lastBranchName = workspace?.name ?? null; + state.latestDescriptorStateKey = descriptorStateKey(workspace); + state.lastBranchName = workspace?.name ?? null; } function removeForCwd(cwd: string): void { const normalizedCwd = resolve(cwd); + const target = watchTargets.get(normalizedCwd); + for (const workspaceId of target?.workspaceIds ?? []) { + workspaceStates.delete(workspaceId); + } watchTargets.delete(normalizedCwd); subscriptions.get(normalizedCwd)?.(); subscriptions.delete(normalizedCwd); } + function removeForWorkspaceId(workspaceId: string): void { + const state = workspaceStates.get(workspaceId); + if (!state) { + return; + } + workspaceStates.delete(workspaceId); + const target = watchTargets.get(state.cwd); + target?.workspaceIds.delete(workspaceId); + if (target?.workspaceIds.size === 0) { + removeForCwd(state.cwd); + } + } + function handleBranchSnapshot(cwd: string, branchName: string | null): void { const target = watchTargets.get(resolve(cwd)); if (!target) { return; } - const previousBranchName = target.lastBranchName; - if (branchName === previousBranchName) { - return; + for (const workspaceId of target.workspaceIds) { + const state = workspaceStates.get(workspaceId); + if (!state) { + continue; + } + const previousBranchName = state.lastBranchName; + if (branchName === previousBranchName) { + continue; + } + state.lastBranchName = branchName; + onBranchChanged?.(workspaceId, previousBranchName, branchName); } - - target.lastBranchName = branchName; - onBranchChanged?.(target.workspaceId, previousBranchName, branchName); } function syncObserver(cwd: string, options: { isGit: boolean; workspaceId: string }): void { const normalizedCwd = resolve(cwd); + const currentState = workspaceStates.get(options.workspaceId); + if (currentState && currentState.cwd !== normalizedCwd) { + removeForWorkspaceId(options.workspaceId); + } if (!options.isGit) { - removeForCwd(normalizedCwd); + removeForWorkspaceId(options.workspaceId); return; } + const target = watchTargets.get(normalizedCwd) ?? { + workspaceIds: new Set(), + }; + watchTargets.set(normalizedCwd, target); + target.workspaceIds.add(options.workspaceId); + if (!workspaceStates.has(options.workspaceId)) { + workspaceStates.set(options.workspaceId, { + cwd: normalizedCwd, + latestDescriptorStateKey: null, + lastBranchName: null, + }); + } + if (subscriptions.has(normalizedCwd)) { return; } - const target: WorkspaceGitWatchTarget = { - cwd: normalizedCwd, - workspaceId: options.workspaceId, - latestDescriptorStateKey: null, - lastBranchName: null, - }; - watchTargets.set(normalizedCwd, target); - - const subscription = workspaceGitService.registerWorkspace( - { cwd: normalizedCwd }, - (snapshot) => { + let subscription: ReturnType; + try { + subscription = workspaceGitService.registerWorkspace({ cwd: normalizedCwd }, (snapshot) => { handleBranchSnapshot(normalizedCwd, snapshot.git.currentBranch ?? null); void emitWorkspaceUpdateForCwd(normalizedCwd).catch((error) => { logger.warn( @@ -152,8 +178,11 @@ export function createWorkspaceGitObserverService(deps: { ); }); emitStatusUpdate(normalizedCwd, snapshot); - }, - ); + }); + } catch (error) { + removeForWorkspaceId(options.workspaceId); + throw error; + } subscriptions.set(normalizedCwd, subscription.unsubscribe); } @@ -163,7 +192,7 @@ export function createWorkspaceGitObserverService(deps: { isGit: workspace.workspaceKind !== "directory", workspaceId: workspace.id, }); - rememberDescriptorState(workspace.workspaceDirectory, workspace); + rememberDescriptorState(workspace.id, workspace); } } @@ -182,24 +211,24 @@ export function createWorkspaceGitObserverService(deps: { }, shouldSkipUpdate(workspaceId, workspace) { - const target = resolveTargetByWorkspaceId(workspaceId); - if (!target) { + const state = workspaceStates.get(workspaceId); + if (!state) { return false; } const nextStateKey = descriptorStateKey(workspace); - if (target.latestDescriptorStateKey === nextStateKey) { + if (state.latestDescriptorStateKey === nextStateKey) { return true; } - target.latestDescriptorStateKey = nextStateKey; + state.latestDescriptorStateKey = nextStateKey; return false; }, recordDescriptorState(workspaceId, nextWorkspace) { - const target = resolveTargetByWorkspaceId(workspaceId); - if (target && onBranchChanged) { + const state = workspaceStates.get(workspaceId); + if (state && onBranchChanged) { const newBranchName = nextWorkspace?.name ?? null; - if (newBranchName !== target.lastBranchName) { - onBranchChanged(workspaceId, target.lastBranchName, newBranchName); + if (newBranchName !== state.lastBranchName) { + onBranchChanged(workspaceId, state.lastBranchName, newBranchName); } } rememberDescriptorState(workspaceId, nextWorkspace); @@ -207,14 +236,7 @@ export function createWorkspaceGitObserverService(deps: { handleBranchSnapshot, - removeForWorkspaceId(workspaceId) { - const target = resolveTargetByWorkspaceId(workspaceId); - if (target) { - removeForCwd(target.cwd); - } - }, - - removeForCwd, + removeForWorkspaceId, dispose() { for (const unsubscribe of subscriptions.values()) { @@ -222,6 +244,7 @@ export function createWorkspaceGitObserverService(deps: { } subscriptions.clear(); watchTargets.clear(); + workspaceStates.clear(); }, }; }