From c8e5519681e442e2c59d2f01044ecaefc8087747 Mon Sep 17 00:00:00 2001 From: Neil Date: Wed, 2 Sep 2026 12:55:31 -0700 Subject: [PATCH] fix(startup): restore the startup-ordering oracle and keep a connected background SSH target undeferred app-startup-routing.test.ts pinned the old step names, so the two ordering cases went vacuous-then-red when the barrier split. Repoint them at the steps that now carry the same fences: 'git-environment-barrier-await' (shell PATH + managed WSL, the fence host Git needs) before hydration worktrees, and 'prepare-terminal-startup-restoration' (which awaits firstWindowStartupServicesReady in main) before terminal reconnect. Both still fail against main's hydration source. Also: the timed-out-eager rewrite of the deferred list re-added background targets that had already connected, undoing removeDeferredSshReconnectTarget and sending fresh panes on a reachable host down the cold-restore path. --- src/renderer/src/app-startup-routing.test.ts | 25 +++++++++++++++---- .../startup-ssh-connection-restore.test.ts | 23 +++++++++++++++++ .../startup/startup-ssh-connection-restore.ts | 9 ++++++- 3 files changed, 51 insertions(+), 6 deletions(-) diff --git a/src/renderer/src/app-startup-routing.test.ts b/src/renderer/src/app-startup-routing.test.ts index 24a9cc75120..d1aad35829a 100644 --- a/src/renderer/src/app-startup-routing.test.ts +++ b/src/renderer/src/app-startup-routing.test.ts @@ -68,8 +68,11 @@ describe('renderer startup runtime routing', () => { const hydrationWorktreesIndex = source.indexOf( "timeRendererStartupStep('fetch-hydration-worktrees'" ) - const servicesIndex = source.indexOf( - "timeRendererStartupStep('first-window-services-await'", + // Why this barrier: worktree hydration can spawn host Git, so it must sit behind the + // shell-PATH + managed-WSL fence. On packaged Windows the window opens before + // shellPathReady resolves, so this really is the fence, not a formality. + const gitEnvironmentBarrierIndex = source.indexOf( + "timeRendererStartupStep('git-environment-barrier-await'", sessionIndex ) const fullWorktreesIndex = source.indexOf('await actions.fetchAllWorktrees()') @@ -89,8 +92,11 @@ describe('renderer startup runtime routing', () => { expect(localReposIndex).toBeLessThan(localGroupsIndex) expect(localGroupsIndex).toBeLessThan(localFoldersIndex) expect(localReposIndex).toBeLessThan(sessionIndex) - expect(sessionIndex).toBeLessThan(servicesIndex) - expect(servicesIndex).toBeLessThan(hydrationWorktreesIndex) + expect(sessionIndex).toBeLessThan(gitEnvironmentBarrierIndex) + expect(gitEnvironmentBarrierIndex).toBeLessThan(hydrationWorktreesIndex) + expect(source.slice(gitEnvironmentBarrierIndex, hydrationWorktreesIndex)).toContain( + 'window.api.app.awaitGitEnvironmentStartupBarrier()' + ) const hydrationWorktreeBlock = source.slice( hydrationWorktreesIndex, source.indexOf('await keybindingsPromise') @@ -180,7 +186,13 @@ describe('renderer startup runtime routing', () => { it('waits for first-window startup services before terminal reconnect', () => { const source = readSource(STARTUP_HYDRATION_PATH) - const servicesIndex = source.indexOf("timeRendererStartupStep('first-window-services-await'") + // Why this step: `app:prepareTerminalStartupRestoration` awaits + // firstWindowStartupServicesReady + managedWslCliStartupBarrierReady in main before it + // does anything else, so it is the renderer-side position of that fence. + // `desktop-startup-ordering.test.ts` pins the main-side await itself. + const servicesIndex = source.indexOf( + "timeRendererStartupStep('prepare-terminal-startup-restoration'" + ) const preReconnectRecoveryIndex = source.indexOf( "timeRendererStartupStep('recover-legacy-worker-terminals-pre-reconnect'" ) @@ -193,6 +205,9 @@ describe('renderer startup runtime routing', () => { ) expect(servicesIndex).toBeGreaterThanOrEqual(0) + expect(source.slice(servicesIndex)).toContain( + 'window.api.app.prepareTerminalStartupRestoration()' + ) expect(preReconnectRecoveryIndex).toBeGreaterThan(servicesIndex) expect(capabilityRefreshIndex).toBeGreaterThan(preReconnectRecoveryIndex) expect(reconnectIndex).toBeGreaterThan(capabilityRefreshIndex) diff --git a/src/renderer/src/startup/startup-ssh-connection-restore.test.ts b/src/renderer/src/startup/startup-ssh-connection-restore.test.ts index 74973a3197b..3464816d0d8 100644 --- a/src/renderer/src/startup/startup-ssh-connection-restore.test.ts +++ b/src/renderer/src/startup/startup-ssh-connection-restore.test.ts @@ -145,6 +145,29 @@ describe('restoreSshConnectionsForStartup', () => { ) }) + it('does not push a connected background target back into the deferred list', async () => { + installWindowApi([target('ssh-active'), target('ssh-bg')]) + // The active host never answers and times out; the background host connects first. + harness.connect.mockImplementation((targetId: string) => + targetId === 'ssh-bg' + ? Promise.resolve(connectedState(targetId)) + : new Promise(() => {}) + ) + + await restoreSshConnectionsForStartup({ + connectionIds: ['ssh-active', 'ssh-bg'], + blockingConnectionIds: ['ssh-active'], + setDeferredSshReconnectTargets: harness.setDeferredSshReconnectTargets, + removeDeferredSshReconnectTarget: harness.removeDeferredSshReconnectTarget, + publishSshConnectionState: harness.publishSshConnectionState + }) + + expect(harness.removeDeferredSshReconnectTarget).toHaveBeenCalledWith('ssh-bg') + // The timed-out rewrite must not resurrect the reachable background target: a deferred + // connected target sends fresh panes down the cold-restore path instead of the normal one. + expect(harness.setDeferredSshReconnectTargets).toHaveBeenLastCalledWith(['ssh-active']) + }, 30_000) + it('keeps passphrase targets deferred and never dials them', async () => { installWindowApi([target('ssh-key', true), target('ssh-bg')]) harness.connect.mockResolvedValue(connectedState('ssh-bg')) diff --git a/src/renderer/src/startup/startup-ssh-connection-restore.ts b/src/renderer/src/startup/startup-ssh-connection-restore.ts index f79359aa727..35efaed2bfa 100644 --- a/src/renderer/src/startup/startup-ssh-connection-restore.ts +++ b/src/renderer/src/startup/startup-ssh-connection-restore.ts @@ -52,6 +52,9 @@ export async function restoreSshConnectionsForStartup(args: { setDeferredSshReconnectTargets(deferredTargetIds) } + // Why tracked: the timed-out branch below rewrites the whole deferred list, and a + // background target that already connected must not be pushed back into it. + const connectedBackgroundTargetIds = new Set() // Why fired before the awaited group: a background target that lands before terminal // reconnect reads as an ordinary connected target, exactly as it does today. for (const { targetId } of backgroundTargets) { @@ -63,6 +66,7 @@ export async function restoreSshConnectionsForStartup(args: { if (state.status === 'connected') { // Why: a still-deferred connected target sends fresh panes down the deferred // spawn path instead of the normal one. Clear it as soon as it is reachable. + connectedBackgroundTargetIds.add(id) removeDeferredSshReconnectTarget(id) } }, @@ -100,7 +104,10 @@ export async function restoreSshConnectionsForStartup(args: { } ) if (timedOutTargets.length > 0) { - setDeferredSshReconnectTargets([...deferredTargetIds, ...timedOutTargets]) + setDeferredSshReconnectTargets([ + ...deferredTargetIds.filter((id) => !connectedBackgroundTargetIds.has(id)), + ...timedOutTargets + ]) } // Why: older/wrapped providers may return no state from connect; poll main once as a compatibility fallback before terminal restoration.