Merge pull request #3391 from kev1n77/fmy/bugfix

fix(git): reduce remote polling overhead and redundant state events

Author: kev1n77

src/web-ui/src/app/components/NavPanel/sections/workspaces/workspaceGitRefreshOptions.test.ts

@@ -62,9 +62,11 @@ describe('getWorkspaceGitBasicInfoOptions', () => {
       });
   });
 
-  it('keeps remote workspace rows on the existing default git refresh behavior', () => {
+  it('defers inactive remote workspace rows until activation too', () => {
     expect(getWorkspaceGitBasicInfoOptions(createWorkspace(WorkspaceKind.Remote), false))
-      .toBeUndefined();
+      .toMatchObject({ isActive: false, refreshOnMount: false, refreshOnActive: true });
+    expect(getWorkspaceGitBasicInfoOptions(createWorkspace(WorkspaceKind.Remote), true))
+      .toMatchObject({ isActive: true, refreshOnMount: true, refreshOnActive: true });
   });
 
   it('suppresses only mount refresh during a history session transition', () => {

src/web-ui/src/app/components/NavPanel/sections/workspaces/workspaceGitRefreshOptions.ts

@@ -1,4 +1,4 @@
-import { isRemoteWorkspace, type WorkspaceInfo } from '@/shared/types';
+import type { WorkspaceInfo } from '@/shared/types';
 import type { GitBasicInfoOptions } from '@/tools/git/hooks/useGitState';
 
 export const WORKSPACE_GIT_BASIC_INFO_SOURCE = 'workspace_item_git_basic_info';
@@ -10,13 +10,9 @@ export const WORKSPACE_GIT_PENDING_CANCEL_SOURCES = [
 export const WORKSPACE_GIT_PENDING_CANCEL_REASONS = ['mount', 'visibility'] as const;
 
 export function getWorkspaceGitBasicInfoOptions(
-  workspace: WorkspaceInfo,
+  _workspace: WorkspaceInfo,
   isActive: boolean
 ): GitBasicInfoOptions | undefined {
-  if (isRemoteWorkspace(workspace)) {
-    return undefined;
-  }
-
   return {
     isActive,
     refreshOnMount: isActive,

src/web-ui/src/tools/git/AGENTS.md

@@ -0,0 +1,16 @@
+# Git state and refresh verification
+
+For changes to Git state subscriptions, polling, refresh scheduling, or workspace
+row activation, run the focused contracts from the repository root:
+
+```bash
+pnpm --dir src/web-ui run test:run src/tools/git/state/GitStateManager.test.ts src/tools/git/hooks/useGitState.test.tsx src/tools/git/services/GitService.test.ts src/tools/git/services/WorkspaceGitInitializer.test.ts src/app/components/NavPanel/sections/workspaces/workspaceGitRefreshOptions.test.ts
+```
+
+Subscriptions observe state; only active consumers register polling demand.
+Keep remote polling conservative, serialize background polls per SSH connection,
+and preserve manual/operation refreshes during retry backoff. Unchanged refreshes
+must not emit state-change events. Retain repository trust error/recovery behavior.
+
+These tests mock host Git calls. Report live SSH and peer-device verification
+separately; fake timers and mocked transports are not remote integration evidence.

src/web-ui/src/tools/git/hooks/useGitState.test.tsx

@@ -10,6 +10,7 @@ import { useGitState } from './useGitState';
 const gitStateManagerMock = vi.hoisted(() => ({
   getState: vi.fn(() => null),
   subscribe: vi.fn(() => vi.fn()),
+  registerPolling: vi.fn(() => vi.fn()),
   registerWindowFocusRefresh: vi.fn(() => vi.fn()),
   refresh: vi.fn(() => Promise.resolve()),
   cancelPendingRefresh: vi.fn(() => false),
@@ -67,6 +68,8 @@ describe('useGitState visibility refresh', () => {
     });
 
     expect(gitStateManagerMock.refresh).not.toHaveBeenCalled();
+    expect(gitStateManagerMock.subscribe).toHaveBeenCalledTimes(1);
+    expect(gitStateManagerMock.registerPolling).not.toHaveBeenCalled();
     expect(gitStateManagerMock.registerWindowFocusRefresh).not.toHaveBeenCalled();
 
     await act(async () => {
@@ -83,6 +86,13 @@ describe('useGitState visibility refresh', () => {
       },
     );
     expect(gitStateManagerMock.registerWindowFocusRefresh).toHaveBeenCalledTimes(1);
+    expect(gitStateManagerMock.registerPolling).toHaveBeenCalledTimes(1);
+    const releasePolling = gitStateManagerMock.registerPolling.mock.results[0].value;
+    await act(async () => {
+      root.render(<GitStateHarness isActive={false} />);
+    });
+    expect(releasePolling).toHaveBeenCalledTimes(1);
+    expect(gitStateManagerMock.subscribe).toHaveBeenCalledTimes(1);
   });
 
   it('cancels and replays active automatic refreshes when mount refresh is temporarily suppressed', async () => {

src/web-ui/src/tools/git/hooks/useGitState.ts

@@ -122,6 +122,11 @@ export function useGitState(options: UseGitStateOptions): UseGitStateReturn {
 
   const isFirstMountRef = useRef(true);
 
+  useEffect(() => {
+    if (!normalizedPath.workspaceId || !isActive) return;
+    return gitStateManager.registerPolling(normalizedPath);
+  }, [isActive, normalizedPath]);
+
   useEffect(() => {
     if (!normalizedPath.workspaceId || !isActive || !participateInWindowFocusRefresh) {
       return;

src/web-ui/src/tools/git/state/GitStateManager.test.ts

@@ -2,6 +2,16 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import { GitStateManager } from './GitStateManager';
 import type { GitStateLayer } from './types';
+import { globalEventBus } from '@/infrastructure/event-bus';
+
+const workspaceCatalog = vi.hoisted(() => new Map<string, { workspaceKind: string; connectionId?: string }>());
+const peerMode = vi.hoisted(() => ({ active: false }));
+vi.mock('@/infrastructure/services/business/workspaceManager', () => ({
+  workspaceManager: { getState: () => ({ openedWorkspaces: workspaceCatalog }) },
+}));
+vi.mock('@/infrastructure/peer-device/peerModeFlag', () => ({
+  isPeerDeviceModeActive: () => peerMode.active,
+}));
 
 const gitApiMocks = vi.hoisted(() => ({
   isGitRepository: vi.fn(),
@@ -59,6 +69,10 @@ describe('GitStateManager refresh performance guards', () => {
 
   beforeEach(() => {
     vi.useFakeTimers();
+    workspaceCatalog.clear();
+    workspaceCatalog.set('workspace-1', { workspaceKind: 'normal' });
+    workspaceCatalog.set('workspace-2', { workspaceKind: 'normal' });
+    peerMode.active = false;
     GitStateManager.resetInstance();
     manager = GitStateManager.getInstance();
     manager.setCacheConfig({ basic: 0, status: 0, detailed: 0 });
@@ -103,10 +117,10 @@ describe('GitStateManager refresh performance guards', () => {
     const addListener = vi.spyOn(document, 'addEventListener');
     const removeListener = vi.spyOn(document, 'removeEventListener');
     try {
-      const unsubscribe = manager.subscribe(repositoryPath, () => {});
+      const unsubscribe = manager.registerPolling(repositoryPath);
       hidden.mockReturnValue(true);
       document.dispatchEvent(new Event('visibilitychange'));
-      const unsubscribeOther = manager.subscribe({ workspaceId: 'workspace-2' }, () => {});
+      const unsubscribeOther = manager.registerPolling({ workspaceId: 'workspace-2' });
       await vi.advanceTimersByTimeAsync(4100);
       expect(gitApiMocks.isGitRepository).not.toHaveBeenCalled();
       const listeners = addListener.mock.calls.filter(([type]) => type === 'visibilitychange');
@@ -131,7 +145,7 @@ describe('GitStateManager refresh performance guards', () => {
   it('does not start polling for an initially hidden document', async () => {
     const hidden = vi.spyOn(document, 'hidden', 'get').mockReturnValue(true);
     try {
-      manager.subscribe(repositoryPath, () => {});
+      manager.registerPolling(repositoryPath);
       await vi.advanceTimersByTimeAsync(4100);
       expect(gitApiMocks.isGitRepository).not.toHaveBeenCalled();
       hidden.mockReturnValue(false);
@@ -164,6 +178,171 @@ describe('GitStateManager refresh performance guards', () => {
     });
   });
 
+  it('keeps passive subscriptions free of IO and reference-counts active demand', async () => {
+    manager.subscribe(repositoryPath, () => {});
+    await vi.advanceTimersByTimeAsync(60_000);
+    expect(gitApiMocks.isGitRepository).not.toHaveBeenCalled();
+    const releaseA = manager.registerPolling(repositoryPath);
+    const releaseB = manager.registerPolling(repositoryPath);
+    releaseA();
+    await vi.advanceTimersByTimeAsync(2100);
+    expect(gitApiMocks.getRepositoryBasic).toHaveBeenCalledTimes(1);
+    releaseB();
+    await vi.advanceTimersByTimeAsync(60_000);
+    expect(gitApiMocks.getRepositoryBasic).toHaveBeenCalledTimes(1);
+  });
+
+  it('polls active SSH workspaces slowly and never broadcasts unchanged results', async () => {
+    workspaceCatalog.set('workspace-1', { workspaceKind: 'remote', connectionId: 'ssh-a' });
+    workspaceCatalog.set('workspace-2', { workspaceKind: 'remote', connectionId: 'ssh-b' });
+    for (const workspaceId of ['workspace-1', 'workspace-2', 'workspace-3']) {
+      manager.subscribe({ workspaceId }, () => {});
+    }
+    manager.registerPolling(repositoryPath);
+    await vi.advanceTimersByTimeAsync(29_999);
+    expect(gitApiMocks.isGitRepository).not.toHaveBeenCalled();
+    await vi.advanceTimersByTimeAsync(101);
+    expect(gitApiMocks.getRepositoryBasic).toHaveBeenCalledTimes(1);
+    vi.mocked(globalEventBus.emit).mockClear();
+    gitEventServiceMock.emit.mockClear();
+    await vi.advanceTimersByTimeAsync(36_000);
+    expect(gitApiMocks.getRepositoryBasic).toHaveBeenCalledTimes(2);
+    expect(globalEventBus.emit).not.toHaveBeenCalled();
+    expect(gitEventServiceMock.emit).not.toHaveBeenCalled();
+    expect(gitApiMocks.getStatus).not.toHaveBeenCalled();
+  });
+
+  it('serializes background reads on one SSH connection without starving another repository', async () => {
+    for (const workspaceId of ['workspace-1', 'workspace-2']) {
+      workspaceCatalog.set(workspaceId, { workspaceKind: 'remote', connectionId: 'ssh-a' });
+      manager.registerPolling({ workspaceId });
+    }
+    const read = deferred<any>();
+    gitApiMocks.getRepositoryBasic.mockReturnValueOnce(read.promise);
+    await vi.advanceTimersByTimeAsync(40_000);
+    expect(gitApiMocks.isGitRepository).toHaveBeenCalledTimes(1);
+    read.resolve({ current_branch: 'main' });
+    await vi.advanceTimersByTimeAsync(2100);
+    expect(gitApiMocks.isGitRepository).toHaveBeenCalledTimes(2);
+    expect(gitApiMocks.isGitRepository.mock.calls.map(([scope]) => scope.workspaceId))
+      .toEqual(['workspace-1', 'workspace-2']);
+  });
+
+  it('backs off failures, allows immediate manual recovery and clears the visible error', async () => {
+    workspaceCatalog.set('workspace-1', { workspaceKind: 'remote', connectionId: 'ssh-a' });
+    const seen = vi.fn();
+    manager.subscribe(repositoryPath, seen);
+    manager.registerPolling(repositoryPath);
+    gitApiMocks.isGitRepository.mockRejectedValueOnce(new Error('SSH disconnected'));
+    await vi.advanceTimersByTimeAsync(30_100);
+    expect(seen.mock.lastCall?.[0].error).toBe('SSH disconnected');
+    await vi.advanceTimersByTimeAsync(59_000);
+    expect(gitApiMocks.isGitRepository).toHaveBeenCalledTimes(1);
+    const retry = manager.refresh(repositoryPath, { layers: ['basic'], force: true });
+    await vi.advanceTimersByTimeAsync(100);
+    await retry;
+    expect(gitApiMocks.isGitRepository).toHaveBeenCalledTimes(2);
+    expect(seen.mock.lastCall?.[0].error).toBeNull();
+    await vi.advanceTimersByTimeAsync(33_000);
+    expect(gitApiMocks.isGitRepository).toHaveBeenCalledTimes(3);
+  });
+
+  it('preserves peer-device polling suppression and uses conservative cadence without metadata', async () => {
+    workspaceCatalog.clear();
+    manager.registerPolling(repositoryPath);
+    await vi.advanceTimersByTimeAsync(6000);
+    expect(gitApiMocks.isGitRepository).not.toHaveBeenCalled();
+    peerMode.active = true;
+    await vi.advanceTimersByTimeAsync(60_000);
+    expect(gitApiMocks.isGitRepository).not.toHaveBeenCalled();
+    peerMode.active = false;
+  });
+
+  it('increases offline retry delays and caps them at five minutes', async () => {
+    workspaceCatalog.set('workspace-1', { workspaceKind: 'remote', connectionId: 'ssh-a' });
+    gitApiMocks.isGitRepository.mockRejectedValue(new Error('SSH disconnected'));
+    manager.registerPolling(repositoryPath);
+    await vi.advanceTimersByTimeAsync(30_100);
+    expect(gitApiMocks.isGitRepository).toHaveBeenCalledTimes(1);
+    await vi.advanceTimersByTimeAsync(60_000);
+    expect(gitApiMocks.isGitRepository).toHaveBeenCalledTimes(1);
+    await vi.advanceTimersByTimeAsync(2000);
+    expect(gitApiMocks.isGitRepository).toHaveBeenCalledTimes(2);
+    await vi.advanceTimersByTimeAsync(120_000);
+    expect(gitApiMocks.isGitRepository).toHaveBeenCalledTimes(2);
+    await vi.advanceTimersByTimeAsync(2000);
+    expect(gitApiMocks.isGitRepository).toHaveBeenCalledTimes(3);
+    await vi.advanceTimersByTimeAsync(242_000);
+    expect(gitApiMocks.isGitRepository).toHaveBeenCalledTimes(4);
+    await vi.advanceTimersByTimeAsync(302_000);
+    expect(gitApiMocks.isGitRepository).toHaveBeenCalledTimes(5);
+    await vi.advanceTimersByTimeAsync(302_000);
+    expect(gitApiMocks.isGitRepository).toHaveBeenCalledTimes(6);
+  });
+
+  it('cancels queued polling on deactivation but preserves a merged manual refresh', async () => {
+    const release = manager.registerPolling(repositoryPath);
+    await vi.advanceTimersByTimeAsync(2000);
+    release();
+    await vi.advanceTimersByTimeAsync(100);
+    expect(gitApiMocks.isGitRepository).not.toHaveBeenCalled();
+    const releaseAgain = manager.registerPolling(repositoryPath);
+    await vi.advanceTimersByTimeAsync(2000);
+    const manual = manager.refresh(repositoryPath, { layers: ['basic'], force: true });
+    releaseAgain();
+    await vi.advanceTimersByTimeAsync(100);
+    await manual;
+    expect(gitApiMocks.isGitRepository).toHaveBeenCalledTimes(1);
+  });
+
+  it('drops a queued poll if the document becomes hidden during debounce', async () => {
+    const hidden = vi.spyOn(document, 'hidden', 'get').mockReturnValue(false);
+    try {
+      manager.registerPolling(repositoryPath);
+      await vi.advanceTimersByTimeAsync(2000);
+      hidden.mockReturnValue(true);
+      document.dispatchEvent(new Event('visibilitychange'));
+      await vi.advanceTimersByTimeAsync(100);
+      expect(gitApiMocks.isGitRepository).not.toHaveBeenCalled();
+    } finally {
+      hidden.mockRestore();
+    }
+  });
+
+  it('notifies error recovery even when the branch and files stay unchanged', async () => {
+    const first = manager.refresh(repositoryPath, { layers: ['basic'], force: true });
+    await vi.advanceTimersByTimeAsync(100);
+    await first;
+    const seen = vi.fn();
+    manager.subscribe(repositoryPath, seen, { immediate: false, layers: ['basic'] });
+    gitApiMocks.isGitRepository.mockRejectedValueOnce(new Error('Offline'));
+    const failed = manager.refresh(repositoryPath, { layers: ['basic'], force: true }).catch(() => {});
+    await vi.advanceTimersByTimeAsync(100);
+    await failed;
+    expect(seen.mock.lastCall?.[0].error).toBe('Offline');
+    const recovered = manager.refresh(repositoryPath, { layers: ['basic'], force: true, silent: true });
+    await vi.advanceTimersByTimeAsync(100);
+    await recovered;
+    expect(seen).toHaveBeenCalledTimes(2);
+    expect(seen.mock.lastCall?.[0].error).toBeNull();
+  });
+
+  it('refreshes stale basic data on activation even with the default infinite TTL', async () => {
+    manager.setCacheConfig({ basic: Infinity });
+    const first = manager.refresh(repositoryPath, { layers: ['basic'], force: true });
+    await vi.advanceTimersByTimeAsync(100);
+    await first;
+    await vi.advanceTimersByTimeAsync(30_000);
+    manager.registerPolling(repositoryPath);
+    gitApiMocks.getRepositoryBasic.mockResolvedValueOnce({ current_branch: 'external-change' });
+    const activate = manager.refresh(repositoryPath, { layers: ['basic'], reason: 'visibility' });
+    await vi.advanceTimersByTimeAsync(100);
+    await activate;
+    expect(manager.getState(repositoryPath)?.currentBranch).toBe('external-change');
+    expect(globalEventBus.emit).toHaveBeenLastCalledWith('git:state:changed',
+      expect.objectContaining({ changedLayers: ['basic'] }));
+  });
+
   it('merges duplicate mount refreshes for the same repository and layer', async () => {
     const first = manager.refresh(repositoryPath, {
       layers: ['basic', 'status'],