fix(workspaces): share git watches by cwd

This commit is contained in:
Mohamed Boudra
2026-07-17 11:17:09 +00:00
parent fa1d932645
commit 7c9db91ac8
3 changed files with 134 additions and 74 deletions

View File

@@ -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<void> {
this.workspaceGitObserver.removeForCwd(input.cwd);
this.scriptRuntimeStore?.removeForWorkspace(input.workspaceId);
private async teardownArchivedWorkspace(workspaceId: string): Promise<void> {
this.workspaceGitObserver.removeForWorkspaceId(workspaceId);
this.scriptRuntimeStore?.removeForWorkspace(workspaceId);
}
private async reconcileAndEmitWorkspaceUpdates(): Promise<void> {
@@ -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":

View File

@@ -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();
});

View File

@@ -10,8 +10,11 @@ import type { PersistedWorkspaceRecord } from "../../workspace-registry.js";
const WORKSPACE_GIT_WATCH_REMOVED_STATE_KEY = "__removed__";
interface WorkspaceGitWatchTarget {
workspaceIds: Set<string>;
}
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<string, WorkspaceGitWatchTarget>();
const workspaceStates = new Map<string, WorkspaceGitWatchState>();
const subscriptions = new Map<string, () => 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<string>(),
};
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<WorkspaceGitService["registerWorkspace"]>;
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();
},
};
}