From f015202a03d2a085534f13351f91ab402c2f55ed Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sat, 16 May 2026 16:25:46 -0700 Subject: [PATCH] fix: surface remote agent status during startup (#2123) --- .../remote-managed-hook-installers.ts | 56 ++++++++ src/main/ssh/ssh-relay-session.test.ts | 31 +++-- src/main/ssh/ssh-relay-session.ts | 52 ++++++++ src/renderer/src/hooks/useIpcEvents.test.ts | 126 ++++++++++++++++++ src/renderer/src/hooks/useIpcEvents.ts | 64 +++++++-- 5 files changed, 309 insertions(+), 20 deletions(-) create mode 100644 src/main/agent-hooks/remote-managed-hook-installers.ts diff --git a/src/main/agent-hooks/remote-managed-hook-installers.ts b/src/main/agent-hooks/remote-managed-hook-installers.ts new file mode 100644 index 000000000..8d13ef610 --- /dev/null +++ b/src/main/agent-hooks/remote-managed-hook-installers.ts @@ -0,0 +1,56 @@ +import type { SFTPWrapper } from 'ssh2' +import type { AgentHookInstallStatus } from '../../shared/agent-hook-types' +import { claudeHookService } from '../claude/hook-service' +import { codexHookService } from '../codex/hook-service' +import { geminiHookService } from '../gemini/hook-service' +import { cursorHookService } from '../cursor/hook-service' +import { grokHookService } from '../grok/hook-service' +import { hermesHookService } from '../hermes/hook-service' + +type RemoteManagedHookInstaller = readonly [ + AgentHookInstallStatus['agent'], + (sftp: SFTPWrapper, remoteHome: string) => Promise +] + +const REMOTE_MANAGED_HOOK_INSTALLERS: readonly RemoteManagedHookInstaller[] = [ + ['claude', (sftp, remoteHome) => claudeHookService.installRemote(sftp, remoteHome)], + ['codex', (sftp, remoteHome) => codexHookService.installRemote(sftp, remoteHome)], + ['gemini', (sftp, remoteHome) => geminiHookService.installRemote(sftp, remoteHome)], + ['cursor', (sftp, remoteHome) => cursorHookService.installRemote(sftp, remoteHome)], + ['grok', (sftp, remoteHome) => grokHookService.installRemote(sftp, remoteHome)], + ['hermes', (sftp, remoteHome) => hermesHookService.installRemote(sftp, remoteHome)] +] + +export async function installRemoteManagedAgentHooks( + sftp: SFTPWrapper, + remoteHome: string +): Promise { + const results: AgentHookInstallStatus[] = [] + for (const [agent, install] of REMOTE_MANAGED_HOOK_INSTALLERS) { + try { + const result = await install(sftp, remoteHome) + results.push(result) + if (result.state === 'error') { + console.warn( + `[agent-hooks] Remote ${agent} managed hook install failed for ${result.configPath}: ${ + result.detail ?? 'unknown error' + }` + ) + } + } catch (error) { + // Why: remote hook installation must not block SSH workspace startup. + // A broken agent config or transient SFTP failure should degrade status + // reporting only, while terminals/filesystem/git still come online. + const detail = error instanceof Error ? error.message : String(error) + console.warn(`[agent-hooks] Remote ${agent} managed hook install threw: ${detail}`) + results.push({ + agent, + state: 'error', + configPath: remoteHome, + managedHooksPresent: false, + detail + }) + } + } + return results +} diff --git a/src/main/ssh/ssh-relay-session.test.ts b/src/main/ssh/ssh-relay-session.test.ts index 52660c519..e6d236bd2 100644 --- a/src/main/ssh/ssh-relay-session.test.ts +++ b/src/main/ssh/ssh-relay-session.test.ts @@ -8,8 +8,9 @@ import type { Store } from '../persistence' import type { SshPortForwardManager } from './ssh-port-forward' import { AGENT_HOOK_INSTALL_PLUGINS_METHOD } from '../../shared/agent-hook-relay' -const { muxRequestMock } = vi.hoisted(() => ({ - muxRequestMock: vi.fn() +const { muxRequestMock, installRemoteManagedAgentHooksMock } = vi.hoisted(() => ({ + muxRequestMock: vi.fn(), + installRemoteManagedAgentHooksMock: vi.fn() })) vi.mock('./ssh-relay-deploy', () => ({ @@ -29,6 +30,10 @@ vi.mock('./ssh-channel-multiplexer', () => { } }) +vi.mock('../agent-hooks/remote-managed-hook-installers', () => ({ + installRemoteManagedAgentHooks: installRemoteManagedAgentHooksMock +})) + vi.mock('../providers/ssh-pty-provider', () => ({ isSshPtyNotFoundError: (err: unknown) => (err instanceof Error ? err.message : String(err)).includes('not found'), @@ -127,6 +132,8 @@ describe('SshRelaySession', () => { delete process.env.ORCA_FEATURE_REMOTE_AGENT_HOOKS muxRequestMock.mockReset() muxRequestMock.mockResolvedValue([]) + installRemoteManagedAgentHooksMock.mockReset() + installRemoteManagedAgentHooksMock.mockResolvedValue([]) mockDeploySuccess() vi.mocked(getPtyIdsForConnection).mockReturnValue([]) }) @@ -151,9 +158,14 @@ describe('SshRelaySession', () => { expect(registerSshGitProvider).toHaveBeenCalledWith('target-1', expect.anything()) }) - it('syncs relay-owned plugin assets before registering the SSH PTY provider', async () => { + it('installs remote managed hooks and relay-owned plugin assets before registering the SSH PTY provider', async () => { process.env.ORCA_FEATURE_REMOTE_AGENT_HOOKS = '1' - muxRequestMock.mockResolvedValue({ ok: true }) + muxRequestMock.mockImplementation(async (method: string) => { + if (method === 'session.resolveHome') { + return { resolvedPath: '/home/orca' } + } + return { ok: true } + }) const sftp = { end: vi.fn() } const { mockStore, mockPortForward, getMainWindow } = createMockDeps() const mockConn = { @@ -167,14 +179,15 @@ describe('SshRelaySession', () => { ([method]) => method === AGENT_HOOK_INSTALL_PLUGINS_METHOD ) expect(installPluginsCallIndex).toBeGreaterThanOrEqual(0) + expect(mockConn.sftp).toHaveBeenCalledTimes(1) + expect(installRemoteManagedAgentHooksMock).toHaveBeenCalledWith(sftp, '/home/orca') + expect(sftp.end).toHaveBeenCalledTimes(1) + expect(installRemoteManagedAgentHooksMock.mock.invocationCallOrder[0]).toBeLessThan( + muxRequestMock.mock.invocationCallOrder[installPluginsCallIndex] + ) expect(muxRequestMock.mock.invocationCallOrder[installPluginsCallIndex]).toBeLessThan( vi.mocked(registerSshPtyProvider).mock.invocationCallOrder[0] ) - // Why: connecting to SSH may upload relay-owned plugin source, but must - // not mutate user-owned agent config files. Remote managed-hook install - // belongs behind an explicit per-host user action. - expect(mockConn.sftp).not.toHaveBeenCalled() - expect(sftp.end).not.toHaveBeenCalled() }) it('does not register providers if dispose wins during initial plugin sync', async () => { diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index 7a9f5d4fb..1c0b0b1c7 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -17,6 +17,7 @@ import { SshPtyProvider, isSshPtyNotFoundError } from '../providers/ssh-pty-prov import { SshFilesystemProvider } from '../providers/ssh-filesystem-provider' import { SshGitProvider } from '../providers/ssh-git-provider' import { agentHookServer } from '../agent-hooks/server' +import { installRemoteManagedAgentHooks } from '../agent-hooks/remote-managed-hook-installers' import { AGENT_HOOK_INSTALL_PLUGINS_METHOD, AGENT_HOOK_NOTIFICATION_METHOD, @@ -432,6 +433,11 @@ export class SshRelaySession { return false } + await this.installManagedHooksOnRemote(mux) + if (shouldContinue && !shouldContinue()) { + return false + } + await this.installPluginsOnRelay(mux) if (shouldContinue && !shouldContinue()) { return false @@ -454,6 +460,52 @@ export class SshRelaySession { return true } + // Why: the relay can inject ORCA_AGENT_HOOK_* env into SSH PTYs, but + // hook-script agents (Claude/Codex/Gemini/etc.) still need their config + // files on the remote host to call Orca's managed script. Install those + // configs before registering the PTY provider so newly spawned agent panes + // report status from their first prompt. + private async installManagedHooksOnRemote(mux: SshChannelMultiplexer): Promise { + if (!isRemoteAgentHooksEnabled()) { + return + } + + let remoteHome: string + try { + const result = (await mux.request('session.resolveHome', { path: '~' })) as { + resolvedPath?: unknown + } + if (typeof result.resolvedPath !== 'string' || result.resolvedPath.length === 0) { + console.warn( + `[ssh-relay-session] skipped remote managed hook install for ${this.targetId}: could not resolve remote home` + ) + return + } + remoteHome = result.resolvedPath + } catch (error) { + console.warn( + `[ssh-relay-session] skipped remote managed hook install for ${this.targetId}: ${ + error instanceof Error ? error.message : String(error) + }` + ) + return + } + + let sftp: Awaited> | null = null + try { + sftp = await this.requireReadyConnection().sftp() + await installRemoteManagedAgentHooks(sftp, remoteHome) + } catch (error) { + console.warn( + `[ssh-relay-session] remote managed hook install failed for ${this.targetId}: ${ + error instanceof Error ? error.message : String(error) + }` + ) + } finally { + ;(sftp as { end?: () => void } | null)?.end?.() + } + } + // Why: ship the OpenCode plugin / Pi extension source bodies to the relay // so it can materialize per-PTY overlay dirs and inject OPENCODE_CONFIG_DIR // / PI_CODING_AGENT_DIR into spawn env. The strings change as we add agent diff --git a/src/renderer/src/hooks/useIpcEvents.test.ts b/src/renderer/src/hooks/useIpcEvents.test.ts index a3b5cfb21..c2f39aa8e 100644 --- a/src/renderer/src/hooks/useIpcEvents.test.ts +++ b/src/renderer/src/hooks/useIpcEvents.test.ts @@ -1995,6 +1995,132 @@ describe('useIpcEvents agent status snapshot integration', () => { ) }) + it('applies remote status snapshots while repo ownership is still hydrating', async () => { + const setAgentStatus = vi.fn() + const getSnapshot = vi.fn(() => + Promise.resolve([ + { + paneKey: FUTURE_PANE_KEY, + state: 'working' as const, + prompt: 'remote p', + agentType: 'codex', + worktreeId: 'wt-1', + connectionId: 'ssh-1', + receivedAt: 1_700_000_000_000, + stateStartedAt: 1_699_999_999_000 + } + ]) + ) + + const storeState: StoreLike = buildStoreState({ + setAgentStatus, + workspaceSessionReady: true, + tabsByWorktree: { + 'wt-1': [{ id: 'tab-future', ptyId: 'pty-1', worktreeId: 'wt-1', title: 'SSH Tab' }] + }, + terminalLayoutsByTabId: { + 'tab-future': { + root: { type: 'leaf', leafId: FUTURE_LEAF_ID }, + activeLeafId: FUTURE_LEAF_ID, + expandedLeafId: null + } + }, + repos: [], + worktreesByRepo: {} + }) + + stubReactSyncEffect() + vi.doMock('../store', () => ({ + useAppStore: { + subscribe: vi.fn(() => () => {}), + getState: () => storeState + } + })) + stubAuxiliaryModules() + vi.stubGlobal( + 'window', + buildWindowApi({ + getSnapshot, + onSet: () => () => {} + }) + ) + + const { useIpcEvents } = await import('./useIpcEvents') + + useIpcEvents() + await Promise.resolve() + + expect(setAgentStatus).toHaveBeenCalledTimes(1) + expect(setAgentStatus).toHaveBeenCalledWith( + FUTURE_PANE_KEY, + expect.objectContaining({ state: 'working', prompt: 'remote p', agentType: 'codex' }), + 'SSH Tab', + { updatedAt: 1_700_000_000_000, stateStartedAt: 1_699_999_999_000 } + ) + }) + + it('still rejects remote status events once the pane resolves to a local repo', async () => { + const setAgentStatus = vi.fn() + const onSetListenerRef: { current: ((data: AgentStatusSetData) => void) | null } = { + current: null + } + const storeState: StoreLike = buildStoreState({ + setAgentStatus, + workspaceSessionReady: true, + tabsByWorktree: { + 'wt-1': [{ id: 'tab-future', ptyId: 'pty-1', worktreeId: 'wt-1', title: 'Local Tab' }] + }, + terminalLayoutsByTabId: { + 'tab-future': { + root: { type: 'leaf', leafId: FUTURE_LEAF_ID }, + activeLeafId: FUTURE_LEAF_ID, + expandedLeafId: null + } + }, + repos: [{ id: 'repo-1', connectionId: null }], + worktreesByRepo: { 'repo-1': [{ id: 'wt-1', repoId: 'repo-1' }] } + }) + + stubReactSyncEffect() + vi.doMock('../store', () => ({ + useAppStore: { + subscribe: vi.fn(() => () => {}), + getState: () => storeState + } + })) + stubAuxiliaryModules() + vi.stubGlobal( + 'window', + buildWindowApi({ + onSet: (cb) => { + onSetListenerRef.current = cb + return () => {} + } + }) + ) + + const { useIpcEvents } = await import('./useIpcEvents') + + useIpcEvents() + await Promise.resolve() + if (typeof onSetListenerRef.current !== 'function') { + throw new Error('Expected agentStatus.onSet listener to be registered') + } + + onSetListenerRef.current({ + paneKey: FUTURE_PANE_KEY, + state: 'working', + prompt: 'remote p', + agentType: 'codex', + worktreeId: 'wt-1', + connectionId: 'ssh-1', + receivedAt: 1_700_000_000_000, + stateStartedAt: 1_699_999_999_000 + }) + + expect(setAgentStatus).not.toHaveBeenCalled() + }) + it('tracks ready push events whose paneKey does not resolve to a renderer tab', async () => { const setAgentStatus = vi.fn() const track = vi.fn() diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index 865934de6..f4066d33c 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -1474,7 +1474,8 @@ export function useIpcEvents(): void { if (!payload) { return } - const { exists, title, repoConnectionId } = resolvePaneKey(store, data.paneKey) + const { exists, title, repoConnectionId, repoConnectionResolved, owningWorktreeId } = + resolvePaneKey(store, data.paneKey) if (!exists) { // Why: empty paneKeys are dropped in main before IPC fanout. Reaching // this branch means a non-empty paneKey escaped without a matching @@ -1496,7 +1497,22 @@ export function useIpcEvents(): void { // The IPC contract declares connectionId as required (string | null), // so the undefined branch only fires under dev hot-reload skew where // the renderer bundle is newer than the preload bundle. - if (data.connectionId !== undefined && data.connectionId !== repoConnectionId) { + // Why: startup snapshot replay can beat repo/worktree hydration for SSH + // panes. If the pane is already present and the event's worktreeId + // matches that tab's worktree, accept the status until repo ownership + // becomes available; once ownership is resolved, keep the strict + // connectionId check below. + const canAcceptPendingRemoteOwnership = + data.connectionId !== undefined && + data.connectionId !== null && + !repoConnectionResolved && + data.worktreeId !== undefined && + data.worktreeId === owningWorktreeId + if ( + data.connectionId !== undefined && + data.connectionId !== repoConnectionId && + !canAcceptPendingRemoteOwnership + ) { return } store.setAgentStatus(data.paneKey, payload, title, { @@ -1685,29 +1701,47 @@ export function useIpcEvents(): void { } /** Resolve a paneKey (tabId:leafId) to both a liveness check and the current - * title, and the connectionId of the repo that owns the pane's worktree. + * title, the pane's worktree, and the connectionId of the repo that owns it. * Walks tabsByWorktree to locate the tab, then resolves the owning worktree * and repo via cached selector maps. Used for agent type inference when the * CLI payload omits agentType, plus to drop status updates targeted at panes * whose tabs have already been torn down or whose owning connection is no * longer live (see docs/design/agent-status-over-ssh.md §5). - * Why combined: callers need all three pieces per hook event, and hook + * Why combined: callers need all routing pieces per hook event, and hook * events can fire many times per second during a tool-use run. Bundling * liveness + title + connectionId into one helper keeps the per-event work * in one place and avoids re-deriving the owning repo at the call site. */ function resolvePaneKey( store: ReturnType, paneKey: string -): { exists: boolean; title: string | undefined; repoConnectionId: string | null } { +): { + exists: boolean + title: string | undefined + repoConnectionId: string | null + repoConnectionResolved: boolean + owningWorktreeId: string | undefined +} { const parsed = parsePaneKey(paneKey) if (!parsed) { - return { exists: false, title: undefined, repoConnectionId: null } + return { + exists: false, + title: undefined, + repoConnectionId: null, + repoConnectionResolved: false, + owningWorktreeId: undefined + } } const { tabId, leafId } = parsed const layout = store.terminalLayoutsByTabId?.[tabId] const leafExists = collectLeafIdsInOrder(layout?.root).includes(leafId) if (!leafExists) { - return { exists: false, title: undefined, repoConnectionId: null } + return { + exists: false, + title: undefined, + repoConnectionId: null, + repoConnectionResolved: false, + owningWorktreeId: undefined + } } // Why: replay can remint numeric pane ids, so status title recovery must use // persisted leaf-keyed titles when crossing from hook state into tab state. @@ -1734,16 +1768,24 @@ function resolvePaneKey( } } // Why: ownership lookup is `tab → worktree → repo → repo.connectionId`. - // Treat unknown owner (no matching worktree/repo) as `null` so remote - // events stamped with a string connectionId are dropped by the caller — - // we cannot prove they belong to the currently-live local repo. + // Keep "resolved to a local repo" distinct from "not hydrated yet" so the + // caller can preserve strict filtering after hydration while accepting SSH + // snapshots that arrive during the startup ownership gap. let repoConnectionId: string | null = null + let repoConnectionResolved = false if (owningWorktreeId !== undefined) { const worktree = getWorktreeMapFromState(store).get(owningWorktreeId) if (worktree) { const repo = getRepoMapFromState(store).get(worktree.repoId) + repoConnectionResolved = repo !== undefined repoConnectionId = repo?.connectionId ?? null } } - return { exists, title: paneTitle ?? tabTitle, repoConnectionId } + return { + exists, + title: paneTitle ?? tabTitle, + repoConnectionId, + repoConnectionResolved, + owningWorktreeId + } }