diff --git a/src/renderer/src/store/slices/github-pr-refresh-host-guard.test.ts b/src/renderer/src/store/slices/github-pr-refresh-host-guard.test.ts new file mode 100644 index 000000000..e2a0839f9 --- /dev/null +++ b/src/renderer/src/store/slices/github-pr-refresh-host-guard.test.ts @@ -0,0 +1,123 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { create } from 'zustand' +import { createGitHubSlice } from './github' +import { createHostedReviewSlice } from './hosted-review' +import type { AppState } from '../types' +import { LOCAL_EXECUTION_HOST_ID } from '../../../../shared/execution-host' + +// Why: regression guard for the renderer OOM crash. Enqueuing the local +// `gh:enqueuePRRefresh` for a repo owned by a remote/SSH/runtime host rejects +// with "Access denied: unknown repository path"; a flood of those failures grew +// the renderer heap to the V8 ceiling and crashed it. enqueueGitHubPRRefresh +// must only hit the local handler for local-host repos. +const enqueuePRRefresh = vi.fn().mockResolvedValue(undefined) + +const mockApi = { + gh: { + prForBranch: vi.fn().mockResolvedValue(null), + enqueuePRRefresh, + issue: vi.fn().mockResolvedValue(null) + }, + hostedReview: { forBranch: vi.fn().mockResolvedValue(null) }, + runtimeEnvironments: { call: vi.fn() } +} + +// @ts-expect-error test window mock +globalThis.window = { api: mockApi } + +function createTestStore() { + return create()( + (...a) => + ({ + ...createGitHubSlice(...a), + ...createHostedReviewSlice(...a) + }) as AppState + ) +} + +function seed(store: ReturnType, repo: Record) { + store.setState({ + settings: { activeRuntimeEnvironmentId: null } as never, + repos: [repo], + worktreesByRepo: { + [repo.id as string]: [ + { + id: 'wt-1', + repoId: repo.id, + path: `${repo.path}/wt`, + branch: 'refs/heads/feature', + displayName: 'feature', + isMainWorktree: false, + isBare: false, + isArchived: false, + linkedPR: null, + linkedIssue: null + } + ] + }, + prCache: {}, + issueCache: {}, + hostedReviewCache: {}, + sshConnectionStates: new Map() + } as unknown as Partial) +} + +describe('enqueueGitHubPRRefresh host guard', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('enqueues the local handler for a local-host repo', () => { + const store = createTestStore() + seed(store, { id: 'local-1', path: '/Users/me/code/local-1', name: 'local-1', kind: 'git' }) + + store.getState().enqueueGitHubPRRefresh('wt-1', 'active', 80) + + expect(enqueuePRRefresh).toHaveBeenCalledTimes(1) + }) + + it('enqueues the local handler for a repo with an explicit local executionHostId', () => { + const store = createTestStore() + seed(store, { + id: 'local-2', + path: '/Users/me/code/local-2', + name: 'local-2', + kind: 'git', + executionHostId: LOCAL_EXECUTION_HOST_ID + }) + + store.getState().enqueueGitHubPRRefresh('wt-1', 'active', 80) + + expect(enqueuePRRefresh).toHaveBeenCalledTimes(1) + }) + + it('does NOT enqueue the local handler for a runtime-host repo (the OOM loop)', () => { + const store = createTestStore() + seed(store, { + id: 'rt-1', + path: '/Users/lobster/orca/workspaces/openclaw/imessage-performance', + name: 'imessage-performance', + kind: 'git', + executionHostId: 'runtime:env-1' + }) + + store.getState().enqueueGitHubPRRefresh('wt-1', 'active', 80) + + expect(enqueuePRRefresh).not.toHaveBeenCalled() + }) + + it('does NOT enqueue the local handler for an SSH repo', () => { + const store = createTestStore() + seed(store, { + id: 'ssh-1', + path: '/home/me/code/ssh-1', + name: 'ssh-1', + kind: 'git', + connectionId: 'conn-1' + }) + + store.getState().enqueueGitHubPRRefresh('wt-1', 'active', 80) + + expect(enqueuePRRefresh).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/store/slices/github-pr-refresh-owner-routing.test.ts b/src/renderer/src/store/slices/github-pr-refresh-owner-routing.test.ts new file mode 100644 index 000000000..d53beb704 --- /dev/null +++ b/src/renderer/src/store/slices/github-pr-refresh-owner-routing.test.ts @@ -0,0 +1,279 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { create } from 'zustand' +import { createGitHubSlice } from './github' +import { createHostedReviewSlice } from './hosted-review' +import type { AppState } from '../types' +import type { PRInfo, Repo, Worktree } from '../../../../shared/types' +import { + createCompatibleRuntimeStatusResponseIfNeeded, + type RuntimeEnvironmentCallRequest +} from '../../runtime/runtime-compatibility-test-fixture' +import { clearRuntimeCompatibilityCacheForTests } from '../../runtime/runtime-rpc-client' + +const runtimeEnvironmentCall = vi.fn() +const runtimeEnvironmentTransportCall = vi.fn() +const enqueuePRRefresh = vi.fn().mockResolvedValue(undefined) +const reportVisiblePRRefreshCandidates = vi.fn().mockResolvedValue(true) + +const mockApi = { + gh: { + prForBranch: vi.fn().mockResolvedValue(null), + refreshPRNow: vi.fn().mockResolvedValue({ kind: 'no-pr', fetchedAt: 1 }), + enqueuePRRefresh, + reportVisiblePRRefreshCandidates, + issue: vi.fn().mockResolvedValue(null) + }, + hostedReview: { forBranch: vi.fn().mockResolvedValue(null) }, + runtimeEnvironments: { call: runtimeEnvironmentTransportCall }, + cache: { + getGitHub: vi.fn().mockResolvedValue(null), + setGitHub: vi.fn().mockResolvedValue(undefined) + } +} + +// @ts-expect-error test window mock +globalThis.window = { api: mockApi } + +function resetRuntimeMocks(): void { + clearRuntimeCompatibilityCacheForTests() + runtimeEnvironmentCall.mockReset() + runtimeEnvironmentTransportCall.mockReset() + runtimeEnvironmentTransportCall.mockImplementation((args: RuntimeEnvironmentCallRequest) => { + return createCompatibleRuntimeStatusResponseIfNeeded(args) ?? runtimeEnvironmentCall(args) + }) +} + +function createTestStore() { + return create()( + (...a) => + ({ + ...createGitHubSlice(...a), + ...createHostedReviewSlice(...a) + }) as AppState + ) +} + +function makePR(overrides: Partial = {}): PRInfo { + return { + number: 12, + title: 'Test PR', + state: 'open', + url: 'https://example.com/pr/12', + checksStatus: 'pending', + updatedAt: '2026-03-28T00:00:00Z', + mergeable: 'UNKNOWN', + headSha: 'head-oid', + ...overrides + } +} + +function makeRepo(overrides: Partial & Pick): Repo { + return { + displayName: overrides.id, + badgeColor: 'blue', + addedAt: 1, + kind: 'git', + ...overrides + } +} + +function makeWorktree(repoId: string, branch: string, id = `${repoId}-wt`): Worktree { + return { + id, + repoId, + path: `/worktrees/${id}`, + head: 'head-oid', + branch, + displayName: branch, + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + linkedLinearIssueWorkspaceId: null, + linkedLinearIssueOrganizationUrlKey: null, + isMainWorktree: false, + isBare: false, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 1, + lastActivityAt: 1 + } +} + +function seed( + store: ReturnType, + state: Pick & Partial +): void { + store.setState({ + settings: { activeRuntimeEnvironmentId: null } as AppState['settings'], + groupBy: 'pr-status', + worktreeCardProperties: ['status'], + prCache: {}, + issueCache: {}, + hostedReviewCache: {}, + commentsCache: {}, + sshConnectionStates: new Map(), + ...state + } as unknown as Partial) +} + +describe('GitHub PR refresh owner-host routing', () => { + beforeEach(() => { + vi.clearAllMocks() + resetRuntimeMocks() + }) + + it('routes explicit PR refresh for a runtime-owned repo to its owner while Local desktop is active', async () => { + runtimeEnvironmentCall.mockResolvedValueOnce({ + id: 'rpc-1', + ok: true, + result: makePR({ number: 23 }), + _meta: { runtimeId: 'remote-runtime' } + }) + const store = createTestStore() + const repoPath = '/runtime/repo' + const branch = 'feature/runtime-owner' + seed(store, { + settings: { activeRuntimeEnvironmentId: null } as AppState['settings'], + repos: [ + makeRepo({ + id: 'repo-runtime', + path: repoPath, + executionHostId: 'runtime:env-1' + }) + ], + worktreesByRepo: { + 'repo-runtime': [makeWorktree('repo-runtime', branch, 'wt-runtime')] + } + }) + + store.getState().enqueueGitHubPRRefresh('wt-runtime', 'active', 80) + + await vi.waitFor(() => expect(runtimeEnvironmentCall).toHaveBeenCalledTimes(1)) + expect(enqueuePRRefresh).not.toHaveBeenCalled() + expect(runtimeEnvironmentCall).toHaveBeenCalledWith({ + selector: 'env-1', + method: 'github.prForBranch', + params: { repo: 'repo-runtime', branch, linkedPRNumber: null }, + timeoutMs: 30_000 + }) + }) + + it('keeps connected SSH PR refresh on the local coordinator even when a runtime is focused', () => { + const store = createTestStore() + const repoPath = '/ssh/repo' + const branch = 'feature/ssh' + seed(store, { + settings: { activeRuntimeEnvironmentId: 'env-focused' } as AppState['settings'], + repos: [ + makeRepo({ + id: 'repo-ssh', + path: repoPath, + connectionId: 'ssh-1', + executionHostId: 'ssh:ssh-1' + }) + ], + sshConnectionStates: new Map([ + ['ssh-1', { targetId: 'ssh-1', status: 'connected', error: null, reconnectAttempt: 0 }] + ]), + worktreesByRepo: { + 'repo-ssh': [makeWorktree('repo-ssh', branch, 'wt-ssh')] + } + }) + + store.getState().refreshGitHubForWorktreeIfStale('wt-ssh') + + expect(runtimeEnvironmentCall).not.toHaveBeenCalled() + expect(enqueuePRRefresh).toHaveBeenCalledWith({ + candidate: expect.objectContaining({ + repoId: 'repo-ssh', + repoPath, + branch, + connectionId: 'ssh-1', + connectionState: 'connected' + }), + reason: 'active', + priority: 80 + }) + }) + + it('routes post-push refresh for a runtime-owned repo to its owner while Local desktop is active', async () => { + runtimeEnvironmentCall.mockResolvedValueOnce({ + id: 'rpc-1', + ok: true, + result: makePR({ number: 24 }), + _meta: { runtimeId: 'remote-runtime' } + }) + const store = createTestStore() + const repoPath = '/runtime/repo' + const branch = 'feature/post-push' + seed(store, { + repos: [ + makeRepo({ + id: 'repo-runtime', + path: repoPath, + executionHostId: 'runtime:env-1' + }) + ], + worktreesByRepo: { + 'repo-runtime': [makeWorktree('repo-runtime', branch, 'wt-runtime')] + } + }) + + store.getState().refreshGitHubForWorktree('wt-runtime') + + await vi.waitFor(() => expect(runtimeEnvironmentCall).toHaveBeenCalledTimes(1)) + expect(enqueuePRRefresh).not.toHaveBeenCalled() + expect(runtimeEnvironmentCall).toHaveBeenCalledWith({ + selector: 'env-1', + method: 'github.prForBranch', + params: { repo: 'repo-runtime', branch, linkedPRNumber: null }, + timeoutMs: 30_000 + }) + }) + + it('splits visible candidates between local coordinator and runtime owner', async () => { + runtimeEnvironmentCall.mockResolvedValueOnce({ + id: 'rpc-1', + ok: true, + result: makePR({ number: 25 }), + _meta: { runtimeId: 'remote-runtime' } + }) + const store = createTestStore() + seed(store, { + repos: [ + makeRepo({ id: 'repo-local', path: '/local/repo' }), + makeRepo({ + id: 'repo-runtime', + path: '/runtime/repo', + executionHostId: 'runtime:env-1' + }) + ], + worktreesByRepo: { + 'repo-local': [makeWorktree('repo-local', 'feature/local', 'wt-local')], + 'repo-runtime': [makeWorktree('repo-runtime', 'feature/runtime', 'wt-runtime')] + } + }) + + store.getState().reportVisibleGitHubPRRefreshCandidates(['wt-local', 'wt-runtime'], 123) + + await vi.waitFor(() => expect(runtimeEnvironmentCall).toHaveBeenCalledTimes(1)) + expect(reportVisiblePRRefreshCandidates).toHaveBeenCalledWith({ + candidates: [ + expect.objectContaining({ + repoId: 'repo-local', + repoPath: '/local/repo', + branch: 'feature/local' + }) + ], + generation: 123 + }) + expect(runtimeEnvironmentCall).toHaveBeenCalledWith({ + selector: 'env-1', + method: 'github.prForBranch', + params: { repo: 'repo-runtime', branch: 'feature/runtime', linkedPRNumber: null }, + timeoutMs: 30_000 + }) + }) +}) diff --git a/src/renderer/src/store/slices/github.ts b/src/renderer/src/store/slices/github.ts index 039550b72..cfd032f96 100644 --- a/src/renderer/src/store/slices/github.ts +++ b/src/renderer/src/store/slices/github.ts @@ -130,6 +130,46 @@ function getRuntimeRepoTarget( return repo ? { target, repo } : null } +function getPRRefreshOwnerRuntimeEnvironmentId( + candidate: Pick +): string | null { + const parsed = parseExecutionHostId(candidate.executionHostId) + if (parsed?.kind === 'runtime') { + return parsed.environmentId + } + const cacheScope = candidate.cacheKey.split('::', 1)[0] + const cacheScopeHost = parseExecutionHostId(cacheScope) + return cacheScopeHost?.kind === 'runtime' ? cacheScopeHost.environmentId : null +} + +function getPRRefreshRuntimeRepoTarget( + state: AppState, + candidate: GitHubPRRefreshCandidate +): { target: { kind: 'environment'; environmentId: string }; repo: Repo } | null { + const ownerRuntimeEnvironmentId = getPRRefreshOwnerRuntimeEnvironmentId(candidate) + if (!ownerRuntimeEnvironmentId) { + return null + } + // Why: PR refreshes must follow the repo owner host, not the Active Server + // dropdown. A runtime-owned worktree can be visible while Local desktop is focused. + return getRuntimeRepoTarget( + state, + candidate.repoPath, + state.settings + ? { ...state.settings, activeRuntimeEnvironmentId: ownerRuntimeEnvironmentId } + : ({ activeRuntimeEnvironmentId: ownerRuntimeEnvironmentId } as AppState['settings']) + ) +} + +function shouldEnqueueLocalPRRefresh(candidate: GitHubPRRefreshCandidate): boolean { + // Why: the local PR coordinator owns local git and SSH bridge refreshes, but + // runtime-owned repos and disconnected SSH repos must not hit the IPC crash path. + if (getPRRefreshOwnerRuntimeEnvironmentId(candidate) !== null) { + return false + } + return !candidate.connectionId || candidate.connectionState === 'connected' +} + type GitHubWorkItemRequestContext = { repoId: string repoPath: string @@ -3383,7 +3423,7 @@ export const createGitHubSlice: StateCreator = (s if (!candidate) { return } - if (getRuntimeRepoTarget(state, candidate.repoPath)) { + if (getPRRefreshRuntimeRepoTarget(state, candidate)) { void get().fetchPRForBranch(candidate.repoPath, candidate.branch, { force: bypassesGitHubPRRefreshFreshness(reason), repoId: candidate.repoId, @@ -3394,6 +3434,9 @@ export const createGitHubSlice: StateCreator = (s }) return } + if (!shouldEnqueueLocalPRRefresh(candidate)) { + return + } const enqueue = window.api.gh.enqueuePRRefresh if (enqueue) { void enqueue({ candidate, reason, priority }) @@ -3424,8 +3467,9 @@ export const createGitHubSlice: StateCreator = (s return worktree ? buildPRRefreshCandidate(state, worktree) : null }) .filter((candidate): candidate is GitHubPRRefreshCandidate => candidate !== null) - if (getActiveRuntimeTarget(state.settings).kind === 'environment') { - for (const candidate of candidates) { + const localCandidates: GitHubPRRefreshCandidate[] = [] + for (const candidate of candidates) { + if (getPRRefreshRuntimeRepoTarget(state, candidate)) { void get().fetchPRForBranch(candidate.repoPath, candidate.branch, { repoId: candidate.repoId, worktreeId: candidate.worktreeId, @@ -3433,12 +3477,15 @@ export const createGitHubSlice: StateCreator = (s fallbackPRNumber: candidate.fallbackPRNumber ?? null, fallbackPRSource: candidate.fallbackPRSource ?? null }) + continue + } + if (shouldEnqueueLocalPRRefresh(candidate)) { + localCandidates.push(candidate) } - return } const reportVisible = window.api.gh.reportVisiblePRRefreshCandidates if (reportVisible) { - void reportVisible({ candidates, generation }).catch((err) => { + void reportVisible({ candidates: localCandidates, generation }).catch((err) => { console.warn('Failed to report visible PR refresh candidates:', err) }) } @@ -3733,7 +3780,7 @@ export const createGitHubSlice: StateCreator = (s fallbackPRNumber: candidate.fallbackPRNumber ?? null, fallbackPRSource: candidate.fallbackPRSource ?? null }) - } else { + } else if (shouldEnqueueLocalPRRefresh(candidate)) { void window.api.gh.enqueuePRRefresh?.({ candidate, reason: 'swr', priority: 10 }) } } @@ -3797,7 +3844,7 @@ export const createGitHubSlice: StateCreator = (s if (!worktree.isBare && branch) { const candidate = buildPRRefreshCandidate(get(), worktree) if (candidate) { - if (getRuntimeRepoTarget(get(), candidate.repoPath)) { + if (getPRRefreshRuntimeRepoTarget(get(), candidate)) { void get().fetchPRForBranch(candidate.repoPath, candidate.branch, { force: true, repoId: candidate.repoId, @@ -3806,7 +3853,7 @@ export const createGitHubSlice: StateCreator = (s fallbackPRNumber: candidate.fallbackPRNumber ?? null, fallbackPRSource: candidate.fallbackPRSource ?? null }) - } else { + } else if (shouldEnqueueLocalPRRefresh(candidate)) { void window.api.gh.enqueuePRRefresh?.({ candidate, reason: 'post-push', priority: 100 }) } } @@ -3995,7 +4042,7 @@ export const createGitHubSlice: StateCreator = (s if (shouldRefreshPR && !worktree.isBare && branch) { const candidate = buildPRRefreshCandidate(state, worktree) if (candidate) { - if (getRuntimeRepoTarget(state, candidate.repoPath)) { + if (getPRRefreshRuntimeRepoTarget(state, candidate)) { void get().fetchPRForBranch(candidate.repoPath, candidate.branch, { force: true, repoId: candidate.repoId, @@ -4004,7 +4051,7 @@ export const createGitHubSlice: StateCreator = (s fallbackPRNumber: candidate.fallbackPRNumber ?? null, fallbackPRSource: candidate.fallbackPRSource ?? null }) - } else { + } else if (shouldEnqueueLocalPRRefresh(candidate)) { void window.api.gh.enqueuePRRefresh?.({ candidate, reason: 'active', priority: 80 }) } }