diff --git a/src/main/daemon/daemon-pty-router-routing-safety.test.ts b/src/main/daemon/daemon-pty-router-routing-safety.test.ts index 5b67aa5442c..4bff0575fa7 100644 --- a/src/main/daemon/daemon-pty-router-routing-safety.test.ts +++ b/src/main/daemon/daemon-pty-router-routing-safety.test.ts @@ -90,6 +90,7 @@ describe('DaemonPtyRouter routing safety', () => { await router.discoverLegacySessions() await router.shutdown(sessionId, { immediate: true, keepHistory: true }) expect(router.getSessionRouteState(sessionId)).toBe('unavailable') + expect(router.supportsGitCredentialGuardHost(sessionId)).toBe(false) const spawning = router.spawn({ sessionId, cols: 80, rows: 24 }) current.emitData(sessionId, 'restored frame') diff --git a/src/main/daemon/daemon-pty-router.test.ts b/src/main/daemon/daemon-pty-router.test.ts index 0771d691b69..936eaf9278d 100644 --- a/src/main/daemon/daemon-pty-router.test.ts +++ b/src/main/daemon/daemon-pty-router.test.ts @@ -382,6 +382,22 @@ describe('DaemonPtyRouter', () => { expect(router.supportsGitCredentialGuardHost('legacy-session')).toBe(false) }) + it('keeps an unresolved guard-host hint conservative until async owner resolution', async () => { + const sessionId = 'preserved-current-session' + const current = createAdapter('current') + const legacy = createAdapter('legacy') + vi.mocked(current.probePtyLiveness).mockResolvedValue(true) + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + + expect(router.supportsGitCredentialGuardHost(sessionId)).toBe(false) + await expect(router.spawn({ sessionId, cols: 80, rows: 24 })).resolves.toEqual({ + id: sessionId + }) + + expect(current.spawn).toHaveBeenCalledExactlyOnceWith({ sessionId, cols: 80, rows: 24 }) + expect(legacy.spawn).not.toHaveBeenCalled() + }) + it('routes fresh foreground confirmation to the session-owning daemon', async () => { const current = createAdapter('current', ['current-session']) const legacy = createAdapter('legacy', ['legacy-session']) diff --git a/src/main/daemon/daemon-pty-router.ts b/src/main/daemon/daemon-pty-router.ts index e8cc451c151..46d12268218 100644 --- a/src/main/daemon/daemon-pty-router.ts +++ b/src/main/daemon/daemon-pty-router.ts @@ -74,7 +74,8 @@ export class DaemonPtyRouter implements IPtyProvider { } supportsGitCredentialGuardHost(sessionId?: string): boolean { - return this.routing.ownerOrCurrent(sessionId).supportsGitCredentialGuardHost() + const owner = sessionId ? this.routing.ownerForHint(sessionId) : this.current + return owner?.supportsGitCredentialGuardHost() ?? false } supportsAgentSessionClaims(): boolean { diff --git a/src/main/daemon/degraded-daemon-pty-provider.test.ts b/src/main/daemon/degraded-daemon-pty-provider.test.ts index 17d41ceadd9..c0b90fe0645 100644 --- a/src/main/daemon/degraded-daemon-pty-provider.test.ts +++ b/src/main/daemon/degraded-daemon-pty-provider.test.ts @@ -622,9 +622,54 @@ describe('DegradedDaemonPtyProvider', () => { expect(current.write).not.toHaveBeenCalled() }) + it('releases an in-flight fallback when the colliding daemon changes incarnation', async () => { + const sessionId = 'in-flight-incarnation-collision' + const previous = { pid: 1, startedAtMs: 1, launchNonce: 'previous' } + const replacement = { pid: 2, startedAtMs: 2, launchNonce: 'replacement' } + const current = createDaemonAdapter('daemon') + const fallback = createProvider('fallback') + let identity = previous + let notifyIdentityChange!: (event: DaemonIdentityChangeEvent) => void + vi.mocked(current.getLastAuthenticatedDaemonIdentity).mockImplementation(() => identity) + vi.mocked(current.onDaemonIdentityChanged).mockImplementation((listener) => { + notifyIdentityChange = listener + return () => {} + }) + let resolveSpawn!: (result: PtySpawnResult) => void + vi.mocked(fallback.spawn).mockReturnValueOnce( + new Promise((resolve) => { + resolveSpawn = resolve + }) + ) + const provider = new DegradedDaemonPtyProvider({ current, legacy: [], fallback }) + const data = vi.fn() + provider.onData(data) + + const spawning = provider.spawn({ sessionId, isNewSession: true, cols: 80, rows: 24 }) + current.emitData(sessionId, 'foreign daemon output') + identity = replacement + notifyIdentityChange({ previous, current: replacement }) + fallback.emitData(sessionId, 'fallback first') + resolveSpawn({ id: sessionId }) + await spawning + provider.write(sessionId, 'fallback input') + + expect(data).toHaveBeenCalledExactlyOnceWith({ id: sessionId, data: 'fallback first' }) + expect(fallback.write).toHaveBeenCalledExactlyOnceWith(sessionId, 'fallback input') + }) + it('rejects unknown fallback output after unavailable tombstones are evicted', async () => { const current = createDaemonAdapter('daemon') const fallback = createProvider('fallback') + const previous = { pid: 1, startedAtMs: 1, launchNonce: 'previous' } + const replacement = { pid: 2, startedAtMs: 2, launchNonce: 'replacement' } + let identity = previous + let notifyIdentityChange!: (event: DaemonIdentityChangeEvent) => void + vi.mocked(current.getLastAuthenticatedDaemonIdentity).mockImplementation(() => identity) + vi.mocked(current.onDaemonIdentityChanged).mockImplementation((listener) => { + notifyIdentityChange = listener + return () => {} + }) const provider = new DegradedDaemonPtyProvider({ current, legacy: [], fallback }) for (let index = 0; index <= 1_000; index += 1) { @@ -657,6 +702,18 @@ describe('DegradedDaemonPtyProvider', () => { id: 'explicit-fallback', data: 'safe frame' }) + + current.emitData('explicit-fallback', 'foreign frame') + identity = replacement + notifyIdentityChange({ previous, current: replacement }) + fallback.emitData('explicit-fallback', 'surviving frame') + provider.write('explicit-fallback', 'safe input') + + expect(data).toHaveBeenNthCalledWith(2, { + id: 'explicit-fallback', + data: 'surviving frame' + }) + expect(fallback.write).toHaveBeenCalledExactlyOnceWith('explicit-fallback', 'safe input') }) it('preserves explicit sequence accounting on daemon data events', () => { diff --git a/src/main/daemon/degraded-daemon-session-routing.ts b/src/main/daemon/degraded-daemon-session-routing.ts index c28a6d5404c..fb2b2915f7f 100644 --- a/src/main/daemon/degraded-daemon-session-routing.ts +++ b/src/main/daemon/degraded-daemon-session-routing.ts @@ -4,15 +4,15 @@ import { shutdownDegradedFallbackSessions } from './degraded-daemon-fallback-shu import { DaemonSessionRouteTable } from './daemon-session-route-table' import { DaemonSessionOwnerUnknownError, sameDaemonIncarnation } from './daemon-session-route' import type { DaemonSnapshotAcknowledgementRoutes } from './daemon-snapshot-acknowledgement-routes' -import { DegradedFallbackSessionRoutes } from './degraded-fallback-session-routes' +import { DegradedFallbackSessionRoutes as FallbackRoutes } from './degraded-fallback-session-routes' import { DegradedFallbackSpawnRoutes } from './degraded-fallback-spawn-routes' export class DegradedDaemonSessionRouting { private readonly adapters: DaemonPtyAdapter[] private readonly adapterSet: ReadonlySet private readonly routes: DaemonSessionRouteTable - private readonly fallbackSessions = new Map() - private readonly fallbackRoutes: DegradedFallbackSessionRoutes + private readonly fallbackOwners = new Map() + private readonly fallbackRoutes: FallbackRoutes private readonly fallbackSpawns = new DegradedFallbackSpawnRoutes() private readonly identityUnsubscribes: (() => void)[] = [] @@ -25,7 +25,7 @@ export class DegradedDaemonSessionRouting { this.adapters = [current, ...legacy] this.adapterSet = new Set(this.adapters) this.routes = new DaemonSessionRouteTable(this.adapters) - this.fallbackRoutes = new DegradedFallbackSessionRoutes(this.fallbackSessions, this.routes) + this.fallbackRoutes = new FallbackRoutes(this.fallbackOwners, this.routes, this.fallbackSpawns) for (const adapter of this.adapters) { this.identityUnsubscribes.push( adapter.onDaemonIdentityChanged(({ previous }) => { @@ -51,7 +51,7 @@ export class DegradedDaemonSessionRouting { incarnation ) for (const session of sessions) { - if (this.fallbackSessions.has(session.id)) { + if (this.fallbackOwners.has(session.id)) { this.fallbackRoutes.recordCollision(session.id) } } @@ -95,7 +95,7 @@ export class DegradedDaemonSessionRouting { if (this.fallbackRoutes.hasCollision(sessionId)) { throw new DaemonSessionOwnerUnknownError(sessionId) } - return this.fallbackSessions.get(sessionId) ?? this.routes.resolveOwnerSync(sessionId) + return this.fallbackOwners.get(sessionId) ?? this.routes.resolveOwnerSync(sessionId) } ownerForHint(sessionId: string): IPtyProvider | null { @@ -110,7 +110,7 @@ export class DegradedDaemonSessionRouting { if (this.fallbackRoutes.hasCollision(sessionId)) { return true } - const fallback = this.fallbackSessions.get(sessionId) + const fallback = this.fallbackOwners.get(sessionId) return fallback ? (fallback.hasPty?.(sessionId) ?? true) : this.routes.hasPty(sessionId) } @@ -131,23 +131,28 @@ export class DegradedDaemonSessionRouting { shouldForwardStreamEvent(provider: IPtyProvider, sessionId: string): boolean { const adapter = this.daemonAdapter(provider) if (adapter) { - const shouldForward = this.routes.recordDataOwner(sessionId, adapter) - if (this.fallbackSessions.has(sessionId) || this.fallbackSpawns.hasLiveCandidate(sessionId)) { + if (this.fallbackOwners.has(sessionId)) { + this.routes.recordFreshOwned(sessionId, adapter) this.fallbackRoutes.recordCollision(sessionId) return false } - return shouldForward + const shouldForward = this.routes.recordDataOwner(sessionId, adapter) + if (!this.fallbackSpawns.hasLiveCandidate(sessionId)) { + return shouldForward + } + this.fallbackRoutes.recordCollision(sessionId) + return false } const route = this.routes.get(sessionId) if (!route) { return ( !this.fallbackSpawns.hasExited(sessionId) && - (this.fallbackSessions.get(sessionId) === provider || + (this.fallbackOwners.get(sessionId) === provider || this.fallbackSpawns.isInFlight(sessionId)) ) } if ( - (this.fallbackSessions.get(sessionId) === provider || + (this.fallbackOwners.get(sessionId) === provider || this.fallbackSpawns.hasLiveCandidate(sessionId)) && route.state === 'unavailable' && !this.fallbackRoutes.hasCollision(sessionId) @@ -166,7 +171,7 @@ export class DegradedDaemonSessionRouting { const adapter = this.daemonAdapter(provider) if (!adapter) { const isOwned = - this.fallbackSessions.get(sessionId) === provider || + this.fallbackOwners.get(sessionId) === provider || this.fallbackSpawns.hasLiveCandidate(sessionId) if (!isOwned) { return false @@ -177,7 +182,7 @@ export class DegradedDaemonSessionRouting { return !route || route.state === 'unavailable' } this.snapshotAcks.dropForProducer(sessionId, adapter) - if (this.fallbackSessions.has(sessionId) || this.fallbackSpawns.hasLiveCandidate(sessionId)) { + if (this.fallbackOwners.has(sessionId) || this.fallbackSpawns.hasLiveCandidate(sessionId)) { this.routes.markUnavailable(sessionId, adapter) if (this.routes.get(sessionId)?.state === 'unavailable') { this.fallbackRoutes.clearCollision(sessionId) @@ -236,7 +241,7 @@ export class DegradedDaemonSessionRouting { for (const { adapter, incarnation, alive: sessionIds } of reconciled) { for (const id of sessionIds) { this.routes.recordOwnedIncarnation(id, adapter, incarnation) - if (this.fallbackSessions.has(id)) { + if (this.fallbackOwners.has(id)) { this.fallbackRoutes.recordCollision(id) } } @@ -277,7 +282,7 @@ export class DegradedDaemonSessionRouting { } async shutdownFallbackSessions(): Promise { - const count = await shutdownDegradedFallbackSessions(this.fallbackSessions, this.fallback) + const count = await shutdownDegradedFallbackSessions(this.fallbackOwners, this.fallback) this.fallbackRoutes.pruneSessionEpochs() return count } @@ -307,7 +312,7 @@ export class DegradedDaemonSessionRouting { this.routes.assertFreshSpawnAvailable(opts.sessionId) return this.fallback } - return this.fallbackSessions.get(opts.sessionId) ?? this.routes.resolveOwner(opts.sessionId) + return this.fallbackOwners.get(opts.sessionId) ?? this.routes.resolveOwner(opts.sessionId) } private daemonAdapter(provider: IPtyProvider): DaemonPtyAdapter | null { diff --git a/src/main/daemon/degraded-fallback-session-routes.ts b/src/main/daemon/degraded-fallback-session-routes.ts index aac2642bb69..ac089ac1e2d 100644 --- a/src/main/daemon/degraded-fallback-session-routes.ts +++ b/src/main/daemon/degraded-fallback-session-routes.ts @@ -2,6 +2,7 @@ import type { IPtyProvider } from '../providers/types' import type { DaemonPtyAdapter } from './daemon-pty-adapter' import { sameDaemonIncarnation } from './daemon-session-route' import type { DaemonSessionRouteTable } from './daemon-session-route-table' +import type { DegradedFallbackSpawnRoutes } from './degraded-fallback-spawn-routes' type FallbackLivenessProbe = { provider: IPtyProvider @@ -15,7 +16,8 @@ export class DegradedFallbackSessionRoutes { constructor( private readonly sessions: Map, - private readonly daemonRoutes: DaemonSessionRouteTable + private readonly daemonRoutes: DaemonSessionRouteTable, + private readonly spawns: DegradedFallbackSpawnRoutes ) {} hasCollision(sessionId: string): boolean { @@ -94,7 +96,7 @@ export class DegradedFallbackSessionRoutes { owner: DaemonPtyAdapter, incarnation: ReturnType ): boolean { - if (!this.sessions.has(sessionId)) { + if (!this.sessions.has(sessionId) && !this.spawns.hasLiveCandidate(sessionId)) { return false } const route = this.daemonRoutes.get(sessionId) diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index 2d376aa05c6..f074f5f00a9 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -2883,6 +2883,7 @@ describe('registerPtyHandlers', () => { env: Record envToDelete?: string[] isNewSession?: boolean + sessionId?: string shellOverride?: string terminalWindowsWslDistro?: string | null terminalWindowsPowerShellImplementation?: string @@ -2943,6 +2944,7 @@ describe('registerPtyHandlers', () => { spawnArgs?: { cwd?: string worktreeId?: string + sessionId?: string shellOverride?: string command?: string launchAgent?: TuiAgent @@ -4057,6 +4059,28 @@ describe('registerPtyHandlers', () => { expect(env.GIT_CONFIG_KEY_2).toBe('credential.guiPrompt') }) + it('uses a conservative guard for an unresolved caller-provided daemon session', async () => { + const supportsGuard = vi.fn(() => false) + const options = await daemonSpawnAndGetOptions( + { + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'http.proxy', + GIT_CONFIG_VALUE_0: 'http://proxy.invalid' + }, + undefined, + undefined, + undefined, + { sessionId: 'preserved-session', command: 'claude' }, + supportsGuard + ) + + expect(supportsGuard).toHaveBeenCalledExactlyOnceWith('preserved-session') + expect(options.sessionId).toBe('preserved-session') + expect(options.env.GIT_CONFIG_COUNT).toBe('3') + expect(options.env.GIT_CONFIG_KEY_1).toBe('credential.interactive') + expect(options.env.GIT_CONFIG_KEY_2).toBe('credential.guiPrompt') + }) + it('passes the minted sessionId through to provider.spawn and host env setup', async () => { const daemonSpawn = setupDaemonAdapter() handlers.clear() diff --git a/src/renderer/src/components/settings/ManageSessionsTable.test.tsx b/src/renderer/src/components/settings/ManageSessionsTable.test.tsx new file mode 100644 index 00000000000..97999f15d5b --- /dev/null +++ b/src/renderer/src/components/settings/ManageSessionsTable.test.tsx @@ -0,0 +1,65 @@ +// @vitest-environment happy-dom + +import '@testing-library/jest-dom/vitest' +import { cleanup, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { PtyManagementSession } from '../../../../preload/api-types' +import { TooltipProvider } from '../ui/tooltip' +import { ManageSessionsTable } from './ManageSessionsTable' + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string) => fallback +})) + +function session(incarnationId: string): PtyManagementSession { + return { + sessionId: 'colliding-session', + incarnationId, + state: 'running', + shellState: 'ready', + isAlive: true, + pid: 123, + cwd: '/workspace', + cols: 80, + rows: 24, + createdAt: 1, + protocolVersion: 30 + } +} + +afterEach(() => { + cleanup() + vi.restoreAllMocks() +}) + +describe('ManageSessionsTable', () => { + it('renders same-id daemon incarnations without duplicate React keys', () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + + render( + + + + ) + + expect(screen.getAllByText('colliding-session')).toHaveLength(2) + expect( + consoleError.mock.calls.filter((args) => + args.some((arg) => /same key|unique "key"/i.test(String(arg))) + ) + ).toEqual([]) + }) +}) diff --git a/src/renderer/src/components/settings/ManageSessionsTable.tsx b/src/renderer/src/components/settings/ManageSessionsTable.tsx index ed44276abb3..3ed49f0632e 100644 --- a/src/renderer/src/components/settings/ManageSessionsTable.tsx +++ b/src/renderer/src/components/settings/ManageSessionsTable.tsx @@ -132,7 +132,7 @@ export function ManageSessionsTable({ const rowClickable = tabId !== null return (