From d78de43e4dce97831a6096147a19bbd65d925e04 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 26 Oct 2026 15:53:06 +0100 Subject: [PATCH] fix(server): establish Git identities at observation setup Read-only checkout refreshes should not resolve watcher namespaces. Observation tests await setup and compare cross-workspace fan-out without depending on completion order. --- ...it-service.observation.integration.test.ts | 4 +++- .../workspace-git-service.observation.test.ts | 23 ++++++++++++++----- .../workspace-git-service.primitive.test.ts | 4 ++-- .../src/server/workspace-git-service.ts | 16 ++++++------- 4 files changed, 29 insertions(+), 18 deletions(-) diff --git a/packages/server/src/server/workspace-git-service.observation.integration.test.ts b/packages/server/src/server/workspace-git-service.observation.integration.test.ts index 2fbd81ac29..e2171886a7 100644 --- a/packages/server/src/server/workspace-git-service.observation.integration.test.ts +++ b/packages/server/src/server/workspace-git-service.observation.integration.test.ts @@ -530,6 +530,8 @@ test("a late Git-ignored tree is pruned while tracked changes still notify consu test("Git refresh, command tracing and 10,000 ignored alias events do no synchronous IO", async () => { const { execFile } = await import("node:child_process"); const { promisify } = await import("node:util"); + const { fileURLToPath } = await import("node:url"); + const repositoryRoot = fileURLToPath(new URL("../../../../", import.meta.url)); const source = ` import assert from 'node:assert/strict'; import { mkdtemp, mkdir, writeFile, symlink, rm } from 'node:fs/promises'; @@ -612,7 +614,7 @@ test("Git refresh, command tracing and 10,000 ignored alias events do no synchro source, ], { - cwd: process.cwd(), + cwd: repositoryRoot, env: { ...process.env, GIT_IO_TRACE: tracing ? "1" : "" }, timeout: 30_000, maxBuffer: 4 * 1024 * 1024, diff --git a/packages/server/src/server/workspace-git-service.observation.test.ts b/packages/server/src/server/workspace-git-service.observation.test.ts index f91a96e464..ab93558793 100644 --- a/packages/server/src/server/workspace-git-service.observation.test.ts +++ b/packages/server/src/server/workspace-git-service.observation.test.ts @@ -1189,7 +1189,7 @@ describe("WorkspaceGitService checkout observation", () => { ]); await vi.advanceTimersByTimeAsync(1_000); await vi.waitFor(() => { - expect(getCalledCwds(getCheckoutStatus)).toEqual([WORKTREE_A, WORKTREE_B]); + expect(getCalledCwds(getCheckoutStatus).sort()).toEqual([WORKTREE_A, WORKTREE_B].sort()); }); first.unsubscribe(); @@ -1624,6 +1624,12 @@ describe("WorkspaceGitService checkout observation", () => { const subscription = service.registerWorkspace({ cwd: REPO_CWD }, vi.fn()); await vi.waitFor(() => { + expect(watcher.subscribe).toHaveBeenCalledWith( + GIT_DIR, + expect.any(Function), + expect.any(Object), + ); + expect(getCheckoutStatus).toHaveBeenCalledTimes(1); expect(service.getMetrics().workspaceObservationSetupInFlightCount).toBe(0); expect(service.getMetrics().workspaceRefreshInFlightCount).toBe(0); }); @@ -1642,14 +1648,17 @@ describe("WorkspaceGitService checkout observation", () => { [60_000, 6], ] as const) { await vi.advanceTimersByTimeAsync(elapsedMs); - expect(pollCount()).toBe(expectedPolls); + await vi.waitFor(() => { + expect(pollCount()).toBe(expectedPolls); + expect(service.getMetrics().workspaceRefreshInFlightCount).toBe(0); + }); } isDirty = true; await vi.advanceTimersByTimeAsync(60_000); - expect(pollCount()).toBe(7); + await vi.waitFor(() => expect(pollCount()).toBe(7)); await vi.advanceTimersByTimeAsync(5_000); - expect(pollCount()).toBe(8); + await vi.waitFor(() => expect(pollCount()).toBe(8)); subscription.unsubscribe(); service.dispose(); @@ -1860,8 +1869,10 @@ describe("WorkspaceGitService checkout observation", () => { expect(watcher.subscribe).toHaveBeenCalledTimes(1); }); await vi.advanceTimersByTimeAsync(35_000); - expect(service.getMetrics().workingTreeWatchSetupInFlightCount).toBe(0); - expect(service.getMetrics().workspaceObservationSetupAdmissionActiveCount).toBe(0); + await vi.waitFor(() => { + expect(service.getMetrics().workingTreeWatchSetupInFlightCount).toBe(0); + expect(service.getMetrics().workspaceObservationSetupAdmissionActiveCount).toBe(0); + }); expect(getWatcherSubscribeCallCount(watcher, REPO_CWD)).toBe(1); openedSubscription.resolve({ diff --git a/packages/server/src/server/workspace-git-service.primitive.test.ts b/packages/server/src/server/workspace-git-service.primitive.test.ts index 9a3599740c..1b43026c7d 100644 --- a/packages/server/src/server/workspace-git-service.primitive.test.ts +++ b/packages/server/src/server/workspace-git-service.primitive.test.ts @@ -991,7 +991,7 @@ describe("WorkspaceGitServiceImpl primitive refresh entrypoint", () => { await vi.advanceTimersByTimeAsync(60_000); await flushPromises(); - expect(subscribe).toHaveBeenCalledTimes(2); + await vi.waitFor(() => expect(subscribe).toHaveBeenCalledTimes(2)); expect(getCheckoutStatus).toHaveBeenCalledTimes(1); subscription.unsubscribe(); @@ -2221,7 +2221,7 @@ describe("WorkspaceGitServiceImpl D2 read methods", () => { expect(getCheckoutStatus).toHaveBeenCalledTimes(2); expect(getPullRequestStatus).toHaveBeenCalledTimes(2); - service.dispose(); + await service.dispose(); }); test("onWorkspaceStateMayHaveChanged preserves includeForge when a file watcher fires within the debounce window", async () => { diff --git a/packages/server/src/server/workspace-git-service.ts b/packages/server/src/server/workspace-git-service.ts index a1310c921a..fbc8441f24 100644 --- a/packages/server/src/server/workspace-git-service.ts +++ b/packages/server/src/server/workspace-git-service.ts @@ -1301,6 +1301,11 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { } await this.promoteWorkingTreeWatchTarget(workingTreeTarget, facts.worktreeRoot); const gitDir = facts.absoluteGitDir; + const gitDirPaths = target.gitDirPaths?.matches(gitDir) + ? target.gitDirPaths + : await GitObservationPaths.create(gitDir); + if (!this.isActiveObservedWorkspaceTarget(target)) return; + target.gitDirPaths = gitDirPaths; const repoGitRoot = facts.gitCommonDir ?? (await this.resolveWorkspaceGitRefsRoot(gitDir)); if (!this.isActiveObservedWorkspaceTarget(target)) { return; @@ -1332,15 +1337,8 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { const promise = this.deps .getCheckoutSnapshotFacts(target.cwd, context) - .then(async (facts) => { - const paths = - facts.isGit && facts.absoluteGitDir - ? await GitObservationPaths.create(facts.absoluteGitDir) - : null; - if (!target.closed) { - target.gitDirPaths = paths; - target.latestFacts = facts; - } + .then((facts) => { + if (!target.closed) target.latestFacts = facts; return facts; }) .finally(() => {