mirror of
https://github.com/stablyai/orca.git
synced 2026-10-24 00:04:01 +00:00
Merge remote-tracking branch 'origin/main' into brennanb2025/fix-run-binding-loss
This commit is contained in:
@@ -73,6 +73,7 @@ const result = spawnSync(
|
||||
'tests/e2e/ssh-cold-hydration-gap-tab-seeding.spec.ts',
|
||||
'tests/e2e/ssh-docker-reconnect-pane-restore.spec.ts',
|
||||
'tests/e2e/ssh-external-image-preview.spec.ts',
|
||||
'tests/e2e/ssh-lost-kill-tab-resurrection.spec.ts',
|
||||
'tests/e2e/ssh-pi-compatible-agent-title.spec.ts',
|
||||
'tests/e2e/ssh-port-forward-lifecycle.spec.ts',
|
||||
'tests/e2e/ssh-reconnect-tab-destruction.spec.ts',
|
||||
|
||||
@@ -0,0 +1,165 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import type { WorkspaceSessionState } from '../../../../shared/workspace-session-state-types'
|
||||
import {
|
||||
SSH_SESSION_EXPIRED_ERROR,
|
||||
SshPtyAbsentFromRelayError
|
||||
} from '../../../providers/ssh-pty-errors'
|
||||
import type { Store } from '../../../persistence'
|
||||
import type { IPtyProvider } from '../../../providers/types'
|
||||
import { spawnForStablePane, type StablePaneOwner } from './stable-owner'
|
||||
|
||||
const LEAF = '1b3f2c4d-5e6a-4b7c-8d9e-0f1a2b3c4d5e'
|
||||
const SIBLING_LEAF = '2c4d3e5f-6a7b-4c8d-9e0f-1a2b3c4d5e6f'
|
||||
const WORKTREE = 'worktree-1'
|
||||
const OWNER: StablePaneOwner = {
|
||||
tabId: 'tab-1',
|
||||
leafId: LEAF,
|
||||
ptyId: 'ssh:conn-1@@pty-1',
|
||||
hasPersistedBinding: true
|
||||
}
|
||||
|
||||
function spawnAfterAttachRejection(
|
||||
error: unknown,
|
||||
persistence?: { store: Store; worktreeId: string }
|
||||
): {
|
||||
run: () => ReturnType<typeof spawnForStablePane>
|
||||
spawn: ReturnType<typeof vi.fn>
|
||||
} {
|
||||
const spawn = vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(error)
|
||||
.mockResolvedValueOnce({ id: 'ssh:conn-1@@pty-2', isReattach: false })
|
||||
return {
|
||||
spawn,
|
||||
run: () =>
|
||||
spawnForStablePane({
|
||||
runtime: undefined,
|
||||
provider: { spawn } as unknown as IPtyProvider,
|
||||
spawnOptions: { cols: 80, rows: 24 },
|
||||
owner: OWNER,
|
||||
connectionId: 'conn-1',
|
||||
resolveOwner: () => null,
|
||||
...(persistence ? { store: persistence.store, worktreeId: persistence.worktreeId } : {})
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
/** Real retirement against an in-memory session, so `retireTerminalSurfaceFromPersistence` runs. */
|
||||
function sessionStore(leaves: string[]): { store: Store; read: () => WorkspaceSessionState } {
|
||||
let session = {
|
||||
tabsByWorktree: {
|
||||
[WORKTREE]: [{ id: 'tab-1', worktreeId: WORKTREE, ptyId: 'ssh:conn-1@@pty-1' }]
|
||||
},
|
||||
terminalLayoutsByTabId: {
|
||||
'tab-1': {
|
||||
root: leaves.reduce<Record<string, unknown>>(
|
||||
(node, leafId) =>
|
||||
Object.keys(node).length === 0
|
||||
? { type: 'leaf', leafId }
|
||||
: { type: 'split', direction: 'row', first: node, second: { type: 'leaf', leafId } },
|
||||
{}
|
||||
),
|
||||
activeLeafId: leaves[0],
|
||||
ptyIdsByLeafId: Object.fromEntries(
|
||||
leaves.map((leafId, index) => [leafId, `ssh:conn-1@@pty-${index + 1}`])
|
||||
)
|
||||
}
|
||||
},
|
||||
terminalPtyIncarnationsByPaneKey: {}
|
||||
} as unknown as WorkspaceSessionState
|
||||
return {
|
||||
read: () => session,
|
||||
store: {
|
||||
getWorkspaceSession: () => session,
|
||||
setWorkspaceSession: (next: WorkspaceSessionState) => {
|
||||
session = next
|
||||
},
|
||||
flushOrThrow: () => {}
|
||||
} as unknown as Store
|
||||
}
|
||||
}
|
||||
|
||||
describe('stable pane adoption after the relay reports the PTY absent', () => {
|
||||
it('spawns fresh once the relay has positively answered for that id', async () => {
|
||||
const { run, spawn } = spawnAfterAttachRejection(
|
||||
new SshPtyAbsentFromRelayError(`${SSH_SESSION_EXPIRED_ERROR}: pty-1`)
|
||||
)
|
||||
|
||||
const result = await run()
|
||||
|
||||
expect(spawn).toHaveBeenCalledTimes(2)
|
||||
expect(spawn.mock.calls[0]?.[0]).toMatchObject({ attachOnly: true })
|
||||
expect(spawn.mock.calls[1]?.[0]).not.toHaveProperty('sessionId')
|
||||
expect(result.owner).toBeNull()
|
||||
})
|
||||
|
||||
// A restarted relay renumbers from pty-1, so the message alone cannot distinguish absence from a
|
||||
// lost link — only the class may authorise abandoning the binding.
|
||||
it.each([
|
||||
['an expired session with no relay evidence', `${SSH_SESSION_EXPIRED_ERROR}: pty-1`],
|
||||
['a lost link', 'SSH connection lost, reconnecting...'],
|
||||
['a request timeout', 'Request "pty.attach" timed out after 10000ms']
|
||||
])('keeps the binding and refuses to respawn after %s', async (_label, message) => {
|
||||
const { run, spawn } = spawnAfterAttachRejection(new Error(message))
|
||||
|
||||
await expect(run()).rejects.toThrow(message)
|
||||
expect(spawn).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
// Why a real store: without one the `args.worktreeId` guard short-circuits and the retirement —
|
||||
// which can delete the parent tab and its layout — never runs. "Reconnect lost every tab" is the
|
||||
// regression this subsystem was reverted for twice, so the composition needs its own coverage.
|
||||
describe('with persistence actually reached', () => {
|
||||
it('retires only the absent leaf and leaves the tab and its sibling bound', async () => {
|
||||
const { store, read } = sessionStore([LEAF, SIBLING_LEAF])
|
||||
const { run, spawn } = spawnAfterAttachRejection(
|
||||
new SshPtyAbsentFromRelayError(`${SSH_SESSION_EXPIRED_ERROR}: pty-1`),
|
||||
{ store, worktreeId: WORKTREE }
|
||||
)
|
||||
|
||||
const result = await run()
|
||||
|
||||
expect(spawn).toHaveBeenCalledTimes(2)
|
||||
expect(result.owner).toBeNull()
|
||||
const session = read()
|
||||
expect(session.tabsByWorktree[WORKTREE]?.map((tab) => tab.id)).toEqual(['tab-1'])
|
||||
const layout = session.terminalLayoutsByTabId['tab-1']
|
||||
expect(layout).toBeDefined()
|
||||
expect(layout?.ptyIdsByLeafId?.[SIBLING_LEAF]).toBe('ssh:conn-1@@pty-2')
|
||||
expect(layout?.ptyIdsByLeafId?.[LEAF]).toBeUndefined()
|
||||
})
|
||||
|
||||
// Pins current behaviour rather than blessing it: retiring the LAST leaf drops the tab from
|
||||
// persistence, and the fallback returns owner=null so main does not re-persist a binding — tab
|
||||
// survival then rests entirely on the renderer. If that ever regresses, this is the tripwire.
|
||||
it('drops the tab when the absent leaf was the only one, leaving re-persistence to the renderer', async () => {
|
||||
const { store, read } = sessionStore([LEAF])
|
||||
const { run, spawn } = spawnAfterAttachRejection(
|
||||
new SshPtyAbsentFromRelayError(`${SSH_SESSION_EXPIRED_ERROR}: pty-1`),
|
||||
{ store, worktreeId: WORKTREE }
|
||||
)
|
||||
|
||||
const result = await run()
|
||||
|
||||
expect(spawn).toHaveBeenCalledTimes(2)
|
||||
expect(result.owner).toBeNull()
|
||||
const session = read()
|
||||
expect(session.tabsByWorktree[WORKTREE]).toEqual([])
|
||||
expect(session.terminalLayoutsByTabId['tab-1']).toBeUndefined()
|
||||
})
|
||||
|
||||
it('leaves persistence untouched when the failure is not positive absence', async () => {
|
||||
const { store, read } = sessionStore([LEAF, SIBLING_LEAF])
|
||||
const before = JSON.stringify(read())
|
||||
const { run, spawn } = spawnAfterAttachRejection(
|
||||
new Error('SSH connection lost, reconnecting...'),
|
||||
{ store, worktreeId: WORKTREE }
|
||||
)
|
||||
|
||||
await expect(run()).rejects.toThrow('SSH connection lost')
|
||||
|
||||
expect(spawn).toHaveBeenCalledTimes(1)
|
||||
expect(JSON.stringify(read())).toBe(before)
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -3,7 +3,10 @@ import type { AgentSessionOwnerBinding } from '../../../../shared/agent-session-
|
||||
import { agentSessionOwnerBindingsEqual } from '../../../../shared/claimed-agent-pty-owner'
|
||||
import { addNodePtyRecoveryHint } from '../../../daemon/node-pty-error-hints'
|
||||
import type { Store } from '../../../persistence'
|
||||
import { isSshPtyNotFoundError } from '../../../providers/ssh-pty-errors'
|
||||
import {
|
||||
isSshPtyAbsentFromRelayError,
|
||||
isSshPtyNotFoundError
|
||||
} from '../../../providers/ssh-pty-errors'
|
||||
import type { IPtyProvider } from '../../../providers/types'
|
||||
import { markClaudePtyExited } from '../../../claude-accounts/live-pty-gate'
|
||||
import { ptyIncarnationById, ptyOwnership } from './ownership-state'
|
||||
@@ -54,7 +57,13 @@ export function normalizeNodePtySpawnError(err: unknown): Error {
|
||||
|
||||
export function isPtyAlreadyGoneError(err: unknown): boolean {
|
||||
const message = err instanceof Error ? err.message : String(err)
|
||||
return isSshPtyNotFoundError(err) || /Session not found/i.test(message)
|
||||
return (
|
||||
isSshPtyNotFoundError(err) ||
|
||||
// Why: the reattach path rewrites the relay's wording to SSH_SESSION_EXPIRED and only this
|
||||
// class preserves that the relay itself answered "absent" rather than the link dropping.
|
||||
isSshPtyAbsentFromRelayError(err) ||
|
||||
/Session not found/i.test(message)
|
||||
)
|
||||
}
|
||||
|
||||
export function delay(ms: number): Promise<void> {
|
||||
|
||||
@@ -121,6 +121,13 @@ export const WORKSPACE_SESSION_FIELD_DISPOSITION = {
|
||||
terminalSurfaceTombstonesByPaneKey: {
|
||||
onRepoRemoval: 'prunedByBespokeRule',
|
||||
onTransfer: 'copiedByBespokeRule'
|
||||
},
|
||||
// Residue: keyed by tab id and pruned by neither path, like remoteSessionIdsByTabId. Bounded
|
||||
// anyway -- every write and every pull merge runs it through the TTL and cap in
|
||||
// shared/closed-terminal-tab-tombstones.ts, and a resolved host retires its own entries.
|
||||
closedTerminalTabTombstonesByTabId: {
|
||||
onRepoRemoval: 'notRepoScoped',
|
||||
onTransfer: 'notTransferred'
|
||||
}
|
||||
} as const satisfies Record<keyof WorkspaceSessionState, SessionFieldDisposition>
|
||||
|
||||
|
||||
@@ -10,3 +10,25 @@ export function isSshPtyIdentityMismatchError(error: unknown): boolean {
|
||||
const message = error instanceof Error ? error.message : String(error)
|
||||
return message.includes(SSH_PTY_IDENTITY_MISMATCH_ERROR) || /identity mismatch/i.test(message)
|
||||
}
|
||||
|
||||
/**
|
||||
* A reachable relay answered for this exact PTY id and reported it absent — positive evidence of
|
||||
* absence from the execution host, so `exited` rather than `unverifiable`
|
||||
* (docs/reference/ssh-execution-boundary.md). Deliberately NOT raised for a transport failure, a
|
||||
* request timeout, a disposed multiplexer, an identity mismatch (the id names a live PTY belonging
|
||||
* to another pane), or `restoreRequired` (the PTY is live, only its source stream is not) — none of
|
||||
* those observe the process, and treating them as absence orphans live remote work.
|
||||
*
|
||||
* Carries the same `SSH_SESSION_EXPIRED` message so message-based consumers are unaffected; only
|
||||
* callers that can act on the stronger verdict test the class.
|
||||
*/
|
||||
export class SshPtyAbsentFromRelayError extends Error {
|
||||
constructor(message: string) {
|
||||
super(message)
|
||||
this.name = 'SshPtyAbsentFromRelayError'
|
||||
}
|
||||
}
|
||||
|
||||
export function isSshPtyAbsentFromRelayError(error: unknown): boolean {
|
||||
return error instanceof SshPtyAbsentFromRelayError
|
||||
}
|
||||
|
||||
@@ -0,0 +1,78 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
SSH_PTY_IDENTITY_MISMATCH_ERROR,
|
||||
SSH_SESSION_EXPIRED_ERROR,
|
||||
isSshPtyAbsentFromRelayError
|
||||
} from './ssh-pty-errors'
|
||||
import { SshPtyProvider } from './ssh-pty-provider'
|
||||
|
||||
function providerRejectingAttachWith(error: unknown): SshPtyProvider {
|
||||
const mux = {
|
||||
request: vi.fn().mockRejectedValue(error),
|
||||
notify: vi.fn(),
|
||||
onNotification: vi.fn().mockReturnValue(vi.fn())
|
||||
}
|
||||
return new SshPtyProvider('conn-1', mux as never)
|
||||
}
|
||||
|
||||
async function reattachRejection(error: unknown): Promise<unknown> {
|
||||
const provider = providerRejectingAttachWith(error)
|
||||
return await provider.spawn({ cols: 80, rows: 24, sessionId: 'pty-1' }).then(
|
||||
() => undefined,
|
||||
(rejection: unknown) => rejection
|
||||
)
|
||||
}
|
||||
|
||||
describe('SSH PTY relay absence verdict', () => {
|
||||
it('reports a relay-delivered not-found as positive absence', async () => {
|
||||
const rejection = await reattachRejection(new Error('PTY "pty-1" not found'))
|
||||
|
||||
expect(isSshPtyAbsentFromRelayError(rejection)).toBe(true)
|
||||
expect((rejection as Error).message).toBe(`${SSH_SESSION_EXPIRED_ERROR}: pty-1`)
|
||||
})
|
||||
|
||||
it('does not report absence when the id names a live PTY owned by another pane', async () => {
|
||||
const rejection = await reattachRejection(
|
||||
new Error('PTY "pty-1" not found (identity mismatch)')
|
||||
)
|
||||
|
||||
expect(isSshPtyAbsentFromRelayError(rejection)).toBe(false)
|
||||
// Byte-identical to the pre-change message: only the class is new, so the toast humanizer,
|
||||
// both spawn-execute lease paths, and the renderer's substring checks are untouched.
|
||||
expect((rejection as Error).message).toBe(
|
||||
`${SSH_SESSION_EXPIRED_ERROR}: pty-1 ${SSH_PTY_IDENTITY_MISMATCH_ERROR}`
|
||||
)
|
||||
})
|
||||
|
||||
// Loss of contact never observes the process: each of these must stay `unverifiable`.
|
||||
it.each([
|
||||
['a lost link', 'SSH connection lost, reconnecting...'],
|
||||
['a disposed multiplexer', 'Multiplexer disposed'],
|
||||
['a request timeout', 'Request "pty.attach" timed out after 10000ms']
|
||||
])('does not report absence for %s', async (_label, message) => {
|
||||
const rejection = await reattachRejection(new Error(message))
|
||||
|
||||
expect(isSshPtyAbsentFromRelayError(rejection)).toBe(false)
|
||||
expect((rejection as Error).message).toBe(message)
|
||||
})
|
||||
|
||||
it('does not report absence when the PTY is live but its source needs restoring', async () => {
|
||||
const mux = {
|
||||
request: vi.fn().mockResolvedValue({
|
||||
incarnationId: 'incarnation-1',
|
||||
sourceRecovery: { status: 'restoreRequired', reason: 'checkpointUnavailable' }
|
||||
}),
|
||||
notify: vi.fn(),
|
||||
onNotification: vi.fn().mockReturnValue(vi.fn())
|
||||
}
|
||||
const provider = new SshPtyProvider('conn-1', mux as never)
|
||||
|
||||
const rejection = await provider.spawn({ cols: 80, rows: 24, sessionId: 'pty-1' }).then(
|
||||
() => undefined,
|
||||
(error: unknown) => error
|
||||
)
|
||||
|
||||
expect(isSshPtyAbsentFromRelayError(rejection)).toBe(false)
|
||||
expect((rejection as Error).message).toContain(SSH_SESSION_EXPIRED_ERROR)
|
||||
})
|
||||
})
|
||||
@@ -3,6 +3,7 @@ import { isPtyIncarnationId, type PtyIncarnationId } from '../../shared/pty-inca
|
||||
import {
|
||||
SSH_PTY_IDENTITY_MISMATCH_ERROR,
|
||||
SSH_SESSION_EXPIRED_ERROR,
|
||||
SshPtyAbsentFromRelayError,
|
||||
isSshPtyIdentityMismatchError,
|
||||
isSshPtyNotFoundError
|
||||
} from './ssh-pty-errors'
|
||||
@@ -225,10 +226,16 @@ export async function reattachSshPtySession(args: {
|
||||
// Why: an expired relay lease must be surfaced distinctly so the renderer clears its binding.
|
||||
console.warn(`[ssh-pty] pty.attach FAILED for ${args.sessionId}:`, error)
|
||||
if (isSshPtyNotFoundError(error)) {
|
||||
const mismatchMarker = isSshPtyIdentityMismatchError(error)
|
||||
? ` ${SSH_PTY_IDENTITY_MISMATCH_ERROR}`
|
||||
: ''
|
||||
throw new Error(`${SSH_SESSION_EXPIRED_ERROR}: ${relaySessionId}${mismatchMarker}`)
|
||||
if (isSshPtyIdentityMismatchError(error)) {
|
||||
// The id names a LIVE PTY owned by another pane, so this is not evidence of absence.
|
||||
throw new Error(
|
||||
`${SSH_SESSION_EXPIRED_ERROR}: ${relaySessionId} ${SSH_PTY_IDENTITY_MISMATCH_ERROR}`
|
||||
)
|
||||
}
|
||||
// Why the class: the relay answered for this exact id, so callers holding a pane binding may
|
||||
// retire it and spawn fresh. Plain `SSH_SESSION_EXPIRED` cannot say that — a restarted relay
|
||||
// renumbers from pty-1, so the message alone is indistinguishable from a lost link.
|
||||
throw new SshPtyAbsentFromRelayError(`${SSH_SESSION_EXPIRED_ERROR}: ${relaySessionId}`)
|
||||
}
|
||||
throw error
|
||||
}
|
||||
|
||||
+404
@@ -0,0 +1,404 @@
|
||||
import type * as React from 'react'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { createDeferred, flushAsyncTicks } from './pty-connection-test-async'
|
||||
import {
|
||||
createManager,
|
||||
createMockTransport,
|
||||
createPane,
|
||||
type ConnectCallbacks,
|
||||
type MockTransport
|
||||
} from './pty-connection-test-pane-fixtures'
|
||||
import { buildPaneConnectionDeps } from './pty-connection-test-deps'
|
||||
import { createInitialStoreState } from './pty-connection-test-store-fixtures'
|
||||
import type { StoreState } from './pty-connection-test-store-state'
|
||||
import {
|
||||
installTerminalTestGlobals,
|
||||
restoreTerminalTestGlobals
|
||||
} from './pty-connection-test-environment'
|
||||
|
||||
// Direct-SSH sibling of remote-hidden-output-restore-unavailable-banner.repro.test.ts.
|
||||
//
|
||||
// Reported by a WSL2 user driving remote Coder workspaces over direct SSH:
|
||||
// "[Orca skipped hidden terminal output because main recovery was unavailable.]"
|
||||
// multiple times a day (v1.4.190).
|
||||
//
|
||||
// Topology modeled: a direct-SSH PTY carries an "ssh:<target>@@<relayPtyId>" id,
|
||||
// so isRemoteRuntimePtyId() is FALSE and canUseHiddenOutputSnapshot() routes
|
||||
// recovery through window.api.pty.getMainBufferSnapshot. That handler serializes
|
||||
// main's model of a stream whose bytes crossed the relay, and it answers null
|
||||
// whenever the snapshot cannot be produced in time (ownership settle deadline,
|
||||
// provider acquisition timeout, post-gap provider snapshot requirement).
|
||||
//
|
||||
// Per docs/reference/ssh-execution-boundary.md a null from a remote execution
|
||||
// host is `unverifiable`, never proof the bytes are gone — the same rule the
|
||||
// "remote:" path already honours via the retry/re-arm budget.
|
||||
//
|
||||
// Repro command:
|
||||
// pnpm exec vitest run --config config/vitest.config.ts \
|
||||
// src/renderer/src/components/terminal-pane/direct-ssh-hidden-output-restore-unavailable-banner.test.ts
|
||||
|
||||
const scheduleRuntimeGraphSync = vi.fn()
|
||||
const shouldSeedCacheTimerOnInitialTitle = vi.fn(() => false)
|
||||
const toastInfo = vi.fn()
|
||||
const notifyCodexPaneBoundForStaleSweep = vi.fn()
|
||||
|
||||
let mockStoreState: StoreState
|
||||
let transportFactoryQueue: MockTransport[] = []
|
||||
let createdTransportOptions: Record<string, unknown>[] = []
|
||||
let storeSubscribers: ((state: StoreState) => void)[] = []
|
||||
|
||||
vi.mock('@/runtime/sync-runtime-graph', () => ({
|
||||
scheduleRuntimeGraphSync
|
||||
}))
|
||||
|
||||
vi.mock('@/store', () => ({
|
||||
useAppStore: {
|
||||
getState: () => mockStoreState,
|
||||
subscribe: (listener: (state: StoreState) => void) => {
|
||||
storeSubscribers.push(listener)
|
||||
return () => {
|
||||
storeSubscribers = storeSubscribers.filter((candidate) => candidate !== listener)
|
||||
}
|
||||
}
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/agent-status', async (importOriginal) => {
|
||||
const { buildAgentStatusModuleMock } = await import('./pty-connection-test-environment')
|
||||
return buildAgentStatusModuleMock(await importOriginal<Record<string, unknown>>())
|
||||
})
|
||||
|
||||
vi.mock('./cache-timer-seeding', () => ({
|
||||
shouldSeedCacheTimerOnInitialTitle
|
||||
}))
|
||||
|
||||
vi.mock('sonner', () => ({
|
||||
toast: {
|
||||
info: toastInfo
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/codex-stale-pane-sweep', () => ({
|
||||
notifyCodexPaneBoundForStaleSweep
|
||||
}))
|
||||
|
||||
vi.mock('react', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof React>()
|
||||
return {
|
||||
...actual,
|
||||
useCallback: <T extends (...args: unknown[]) => unknown>(fn: T): T => fn
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('./pty-transport', () => ({
|
||||
createIpcPtyTransport: vi.fn((options: Record<string, unknown>) => {
|
||||
createdTransportOptions.push(options)
|
||||
const nextTransport = transportFactoryQueue.shift()
|
||||
if (!nextTransport) {
|
||||
throw new Error('No mock transport queued')
|
||||
}
|
||||
return nextTransport
|
||||
})
|
||||
}))
|
||||
|
||||
vi.mock('./remote-runtime-pty-transport', () => ({
|
||||
createRemoteRuntimePtyTransport: vi.fn(
|
||||
(_environmentId: string, options: Record<string, unknown>) => {
|
||||
createdTransportOptions.push(options)
|
||||
const nextTransport = transportFactoryQueue.shift()
|
||||
if (!nextTransport) {
|
||||
throw new Error('No mock transport queued')
|
||||
}
|
||||
return nextTransport
|
||||
}
|
||||
)
|
||||
}))
|
||||
|
||||
vi.mock('./pty-dispatcher', async (importOriginal) => {
|
||||
const actual = await importOriginal<Record<string, unknown>>()
|
||||
return {
|
||||
...actual,
|
||||
getEagerPtyBufferHandle: vi.fn(() => undefined)
|
||||
}
|
||||
})
|
||||
|
||||
// ── Scenario identities ─────────────────────────────────────────────────────
|
||||
// Real direct-SSH app PTY id shape (shared/ssh-pty-id.ts).
|
||||
const SSH_PTY_ID = 'ssh:coder-ws@@pty-1'
|
||||
// Overflows the renderer's hidden background queue (2MB lossy cap), dropping the
|
||||
// backlog and latching main-model restore.
|
||||
const HIDDEN_BYTES = 'x'.repeat(2 * 1024 * 1024 + 1)
|
||||
const LIVE_AGENT_CHUNK = 'LIVE_AGENT_CHUNK\r\n'
|
||||
const HOST_SNAPSHOT_MARKER = 'HOST_SNAPSHOT_HIDDEN_AGENT_CONTENT'
|
||||
const HOST_SNAPSHOT = {
|
||||
data: `${HOST_SNAPSHOT_MARKER}\r\n`,
|
||||
cols: 120,
|
||||
rows: 40,
|
||||
seq: HIDDEN_BYTES.length + LIVE_AGENT_CHUNK.length,
|
||||
source: 'headless'
|
||||
}
|
||||
const BANNER_FRAGMENT = 'main recovery was unavailable'
|
||||
|
||||
type HostSnapshot = typeof HOST_SNAPSHOT
|
||||
|
||||
type SshPaneDrive = {
|
||||
transport: MockTransport
|
||||
pane: ReturnType<typeof createPane>
|
||||
deps: ReturnType<typeof buildPaneConnectionDeps>
|
||||
disposable: { dispose: () => void }
|
||||
deliver: (data: string, seq: number) => void
|
||||
writtenChunks: () => string[]
|
||||
}
|
||||
|
||||
function stubMainBufferSnapshot(
|
||||
impl: () => Promise<HostSnapshot | null>
|
||||
): ReturnType<typeof vi.fn> {
|
||||
const getMainBufferSnapshot = vi.fn(impl)
|
||||
;(window.api.pty as unknown as Record<string, unknown>).getMainBufferSnapshot =
|
||||
getMainBufferSnapshot
|
||||
return getMainBufferSnapshot
|
||||
}
|
||||
|
||||
async function connectHiddenDirectSshPane(): Promise<SshPaneDrive> {
|
||||
const { connectPanePty } = await import('./pty-connection')
|
||||
const transport = createMockTransport(SSH_PTY_ID)
|
||||
const capturedDataCallback: {
|
||||
current: ((data: string, meta?: { seq?: number; rawLength?: number }) => void) | null
|
||||
} = { current: null }
|
||||
transport.connect.mockImplementation(async ({ callbacks }: { callbacks?: ConnectCallbacks }) => {
|
||||
capturedDataCallback.current = callbacks?.onData ?? null
|
||||
return SSH_PTY_ID
|
||||
})
|
||||
transportFactoryQueue.push(transport)
|
||||
const pane = createPane(1)
|
||||
const manager = createManager(1)
|
||||
const deps = buildPaneConnectionDeps(() => mockStoreState, {
|
||||
isVisibleRef: { current: false }
|
||||
})
|
||||
const disposable = connectPanePty(pane as never, manager as never, deps as never)
|
||||
await flushAsyncTicks(6)
|
||||
expect(capturedDataCallback.current).not.toBeNull()
|
||||
return {
|
||||
transport,
|
||||
pane,
|
||||
deps,
|
||||
disposable,
|
||||
deliver: (data, seq) => capturedDataCallback.current?.(data, { seq, rawLength: data.length }),
|
||||
writtenChunks: () => pane.terminal.write.mock.calls.map(([data]) => String(data))
|
||||
}
|
||||
}
|
||||
|
||||
function driveHiddenBacklogThenReveal(drive: SshPaneDrive): void {
|
||||
drive.deliver(HIDDEN_BYTES, HIDDEN_BYTES.length)
|
||||
;(drive.deps.isVisibleRef as { current: boolean }).current = true
|
||||
drive.deliver(LIVE_AGENT_CHUNK, HIDDEN_BYTES.length + LIVE_AGENT_CHUNK.length)
|
||||
}
|
||||
|
||||
async function advanceThroughNullRetryBudget(): Promise<void> {
|
||||
for (let retry = 0; retry < 3; retry++) {
|
||||
vi.advanceTimersByTime(50)
|
||||
vi.advanceTimersByTime(0)
|
||||
await flushAsyncTicks(10)
|
||||
}
|
||||
}
|
||||
|
||||
async function allowSelfHealWindow(): Promise<void> {
|
||||
for (let step = 0; step < 10; step += 1) {
|
||||
await vi.advanceTimersByTimeAsync(500)
|
||||
await flushAsyncTicks(20)
|
||||
}
|
||||
}
|
||||
|
||||
function observeFinalPaneState(drive: SshPaneDrive): {
|
||||
rawHiddenBacklogWritten: boolean
|
||||
liveAgentChunkWritten: boolean
|
||||
unavailableBannerWritten: boolean
|
||||
hiddenOutputRecoveredFromHostSnapshot: boolean
|
||||
} {
|
||||
const joined = drive.writtenChunks().join('')
|
||||
return {
|
||||
rawHiddenBacklogWritten: joined.includes('x'.repeat(1024)),
|
||||
liveAgentChunkWritten: joined.includes('LIVE_AGENT_CHUNK'),
|
||||
unavailableBannerWritten: joined.includes(BANNER_FRAGMENT),
|
||||
hiddenOutputRecoveredFromHostSnapshot: joined.includes(HOST_SNAPSHOT_MARKER)
|
||||
}
|
||||
}
|
||||
|
||||
describe('direct-SSH hidden-output restore abandonment', () => {
|
||||
beforeEach(() => {
|
||||
vi.resetModules()
|
||||
vi.clearAllMocks()
|
||||
transportFactoryQueue = []
|
||||
createdTransportOptions = []
|
||||
storeSubscribers = []
|
||||
mockStoreState = createInitialStoreState(() => mockStoreState)
|
||||
installTerminalTestGlobals()
|
||||
// The reporter's client is WSL2, but the banner path is platform-independent;
|
||||
// pin darwin so the Windows synchronized-output scan stays out of the writes.
|
||||
;(window.api.platform as unknown as Record<string, unknown>).get = vi.fn(() => ({
|
||||
platform: 'darwin',
|
||||
osRelease: '25.0.0'
|
||||
}))
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
await restoreTerminalTestGlobals()
|
||||
})
|
||||
|
||||
it('[invariant] must not declare recovery unavailable when the SSH host snapshot is one retry tick away', async () => {
|
||||
const getMainBufferSnapshot: ReturnType<typeof vi.fn> = stubMainBufferSnapshot(async () =>
|
||||
getMainBufferSnapshot.mock.calls.length <= 4 ? null : HOST_SNAPSHOT
|
||||
)
|
||||
const drive = await connectHiddenDirectSshPane()
|
||||
vi.useFakeTimers()
|
||||
driveHiddenBacklogThenReveal(drive)
|
||||
await flushAsyncTicks(20)
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledTimes(1)
|
||||
|
||||
await advanceThroughNullRetryBudget()
|
||||
await allowSelfHealWindow()
|
||||
|
||||
expect(observeFinalPaneState(drive)).toEqual({
|
||||
rawHiddenBacklogWritten: false,
|
||||
liveAgentChunkWritten: true,
|
||||
unavailableBannerWritten: false,
|
||||
hiddenOutputRecoveredFromHostSnapshot: true
|
||||
})
|
||||
drive.disposable.dispose()
|
||||
})
|
||||
|
||||
it('[invariant] must not turn a merely-slow SSH host snapshot into a loss banner and a permanent scrollback gap', async () => {
|
||||
const slowSnapshot = createDeferred<HostSnapshot>()
|
||||
const getMainBufferSnapshot = stubMainBufferSnapshot(async () => HOST_SNAPSHOT)
|
||||
getMainBufferSnapshot.mockReturnValueOnce(slowSnapshot.promise)
|
||||
const drive = await connectHiddenDirectSshPane()
|
||||
vi.useFakeTimers()
|
||||
driveHiddenBacklogThenReveal(drive)
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
// The 750ms foreground deadline elapses while main is still serializing.
|
||||
vi.advanceTimersByTime(750)
|
||||
vi.advanceTimersByTime(0)
|
||||
await flushAsyncTicks(10)
|
||||
slowSnapshot.resolve(HOST_SNAPSHOT)
|
||||
await flushAsyncTicks(20)
|
||||
await allowSelfHealWindow()
|
||||
|
||||
expect(observeFinalPaneState(drive)).toEqual({
|
||||
rawHiddenBacklogWritten: false,
|
||||
liveAgentChunkWritten: true,
|
||||
unavailableBannerWritten: false,
|
||||
hiddenOutputRecoveredFromHostSnapshot: true
|
||||
})
|
||||
drive.disposable.dispose()
|
||||
})
|
||||
|
||||
it('[invariant] a permanently silent SSH host banners once after a bounded number of re-arms and then stops requesting', async () => {
|
||||
const getMainBufferSnapshot = stubMainBufferSnapshot(async () => null)
|
||||
const drive = await connectHiddenDirectSshPane()
|
||||
vi.useFakeTimers()
|
||||
driveHiddenBacklogThenReveal(drive)
|
||||
await flushAsyncTicks(20)
|
||||
await advanceThroughNullRetryBudget()
|
||||
|
||||
expect(drive.writtenChunks().join('')).toContain('LIVE_AGENT_CHUNK')
|
||||
|
||||
for (let step = 0; step < 6; step += 1) {
|
||||
await allowSelfHealWindow()
|
||||
}
|
||||
const bannerCount = drive
|
||||
.writtenChunks()
|
||||
.filter((data) => data.includes(BANNER_FRAGMENT)).length
|
||||
expect(bannerCount).toBe(1)
|
||||
const settledRequests = getMainBufferSnapshot.mock.calls.length
|
||||
expect(settledRequests).toBeGreaterThan(4)
|
||||
expect(settledRequests).toBeLessThan(40)
|
||||
|
||||
for (let step = 0; step < 4; step += 1) {
|
||||
await allowSelfHealWindow()
|
||||
}
|
||||
expect(getMainBufferSnapshot.mock.calls.length).toBe(settledRequests)
|
||||
expect(drive.writtenChunks().filter((data) => data.includes(BANNER_FRAGMENT)).length).toBe(1)
|
||||
drive.disposable.dispose()
|
||||
})
|
||||
|
||||
// The re-arm budget bounds one stall; live bytes are not evidence the host can
|
||||
// answer again. Continuous output on the SAME ssh: id must not re-open recovery
|
||||
// or refresh the budget, or the bounded re-arm becomes a permanent ~2s repaint loop.
|
||||
it('[invariant] continuous live output on a silent SSH host cannot refresh the re-arm budget into a repaint loop', async () => {
|
||||
const getMainBufferSnapshot = stubMainBufferSnapshot(async () => null)
|
||||
const drive = await connectHiddenDirectSshPane()
|
||||
vi.useFakeTimers()
|
||||
driveHiddenBacklogThenReveal(drive)
|
||||
await flushAsyncTicks(20)
|
||||
await advanceThroughNullRetryBudget()
|
||||
|
||||
let seq = HIDDEN_BYTES.length + LIVE_AGENT_CHUNK.length
|
||||
for (let step = 0; step < 60; step += 1) {
|
||||
const chunk = `LIVE_TICK_${step}\r\n`
|
||||
seq += chunk.length
|
||||
drive.deliver(chunk, seq)
|
||||
await vi.advanceTimersByTimeAsync(300)
|
||||
await flushAsyncTicks(20)
|
||||
}
|
||||
|
||||
const joined = drive.writtenChunks().join('')
|
||||
expect(joined).toContain('LIVE_TICK_0')
|
||||
expect(joined).toContain('LIVE_TICK_59')
|
||||
expect(joined.split(BANNER_FRAGMENT).length - 1).toBe(1)
|
||||
const settledRequests = getMainBufferSnapshot.mock.calls.length
|
||||
expect(settledRequests).toBeLessThan(40)
|
||||
|
||||
for (let step = 0; step < 30; step += 1) {
|
||||
const chunk = `LIVE_TAIL_${step}\r\n`
|
||||
seq += chunk.length
|
||||
drive.deliver(chunk, seq)
|
||||
await vi.advanceTimersByTimeAsync(300)
|
||||
await flushAsyncTicks(20)
|
||||
}
|
||||
expect(getMainBufferSnapshot.mock.calls.length).toBe(settledRequests)
|
||||
expect(drive.writtenChunks().join('').split(BANNER_FRAGMENT).length - 1).toBe(1)
|
||||
drive.disposable.dispose()
|
||||
})
|
||||
|
||||
// The re-arm budget bounds ONE stall. It only clears on a successful snapshot or
|
||||
// a PTY change, so without an explicit reset at the banner a single long outage
|
||||
// leaves a long-lived SSH pane permanently at zero tolerance.
|
||||
it('[invariant] a spent re-arm budget must not leave the SSH pane at zero tolerance for its next hidden episode', async () => {
|
||||
const revivedSnapshot = createDeferred<HostSnapshot>()
|
||||
let hostIsSilent = true
|
||||
stubMainBufferSnapshot(async () => (hostIsSilent ? null : revivedSnapshot.promise))
|
||||
const drive = await connectHiddenDirectSshPane()
|
||||
vi.useFakeTimers()
|
||||
driveHiddenBacklogThenReveal(drive)
|
||||
await flushAsyncTicks(20)
|
||||
await advanceThroughNullRetryBudget()
|
||||
for (let step = 0; step < 6; step += 1) {
|
||||
await allowSelfHealWindow()
|
||||
}
|
||||
expect(drive.writtenChunks().filter((data) => data.includes(BANNER_FRAGMENT))).toHaveLength(1)
|
||||
|
||||
// Second episode on the SAME ssh: id: the host is alive again but its snapshot
|
||||
// lands just past the 750ms foreground deadline.
|
||||
hostIsSilent = false
|
||||
let seq = HIDDEN_BYTES.length + LIVE_AGENT_CHUNK.length
|
||||
;(drive.deps.isVisibleRef as { current: boolean }).current = false
|
||||
seq += HIDDEN_BYTES.length
|
||||
drive.deliver(HIDDEN_BYTES, seq)
|
||||
;(drive.deps.isVisibleRef as { current: boolean }).current = true
|
||||
seq += LIVE_AGENT_CHUNK.length
|
||||
drive.deliver(LIVE_AGENT_CHUNK, seq)
|
||||
await flushAsyncTicks(20)
|
||||
vi.advanceTimersByTime(750)
|
||||
vi.advanceTimersByTime(0)
|
||||
await flushAsyncTicks(10)
|
||||
revivedSnapshot.resolve(HOST_SNAPSHOT)
|
||||
await flushAsyncTicks(20)
|
||||
await allowSelfHealWindow()
|
||||
|
||||
const joined = drive.writtenChunks().join('')
|
||||
expect(joined.split(BANNER_FRAGMENT).length - 1).toBe(1)
|
||||
expect(joined).toContain(HOST_SNAPSHOT_MARKER)
|
||||
drive.disposable.dispose()
|
||||
})
|
||||
})
|
||||
+15
-7
@@ -9,6 +9,7 @@ import {
|
||||
} from './hidden-output-restore-limits'
|
||||
import { shouldWritePtyOutputForeground } from './foreground-output-scan'
|
||||
import { isRemoteRuntimePtyId } from './paired-parked-terminal-restore'
|
||||
import { isRemoteExecutionHostPtyId } from './remote-execution-host-pty'
|
||||
|
||||
import type { ConnectPanePtySession } from './connect-pane-pty-session'
|
||||
|
||||
@@ -132,17 +133,24 @@ export function bindHiddenOutputRestoreDrain(session: ConnectPanePtySession): vo
|
||||
}, HIDDEN_OUTPUT_RESTORE_FOREGROUND_TIMEOUT_MS)
|
||||
}
|
||||
|
||||
// Trades the loss banner for one bounded post-suppression repaint from the
|
||||
// host's authoritative buffer. Ordered before the state reset so the repaint
|
||||
// timer arms against the live ptyId (mirrors the flood-abandon call sites).
|
||||
// Trades the loss banner for one bounded post-suppression repaint from whatever
|
||||
// answers for this PTY — the paired runtime's own buffer, or main's model of the
|
||||
// direct-SSH stream. Ordered before the state reset so the repaint timer arms
|
||||
// against the live ptyId (mirrors the flood-abandon call sites).
|
||||
session.rearmRemoteHiddenOutputRestoreInsteadOfWarning = function (
|
||||
ptyId: string,
|
||||
reason: string
|
||||
): boolean {
|
||||
if (
|
||||
!isRemoteRuntimePtyId(ptyId) ||
|
||||
session.hiddenOutputRestoreRemoteAbandonCycles >= HIDDEN_OUTPUT_RESTORE_REMOTE_REARM_MAX
|
||||
) {
|
||||
if (!isRemoteExecutionHostPtyId(ptyId)) {
|
||||
return false
|
||||
}
|
||||
if (session.hiddenOutputRestoreRemoteAbandonCycles >= HIDDEN_OUTPUT_RESTORE_REMOTE_REARM_MAX) {
|
||||
// Why the reset: the budget bounds ONE stall, and returning false ends that
|
||||
// stall with the loss banner. Without this, the counter only ever clears on a
|
||||
// successful snapshot or a PTY change, so a single ~10s outage leaves a
|
||||
// long-lived SSH pane at zero tolerance — every later hidden episode banners
|
||||
// on the first null even though that null is still only `unverifiable`.
|
||||
session.hiddenOutputRestoreRemoteAbandonCycles = 0
|
||||
return false
|
||||
}
|
||||
session.hiddenOutputRestoreRemoteAbandonCycles += 1
|
||||
|
||||
@@ -0,0 +1,15 @@
|
||||
import { parseAppSshPtyId } from '../../../../../shared/ssh-pty-id'
|
||||
|
||||
import { isRemoteRuntimePtyId } from './paired-parked-terminal-restore'
|
||||
|
||||
// Why: hidden-output recovery for both remote shapes is answered across a link —
|
||||
// paired runtimes ("remote:", host-owned buffer) and direct SSH
|
||||
// ("ssh:<target>@@<id>", main's relay-fed model, whose serialize can block on
|
||||
// host RPCs). A null/late snapshot from either is `unverifiable`, never proof
|
||||
// the bytes are gone (docs/reference/ssh-execution-boundary.md).
|
||||
export function isRemoteExecutionHostPtyId(ptyId: string | null | undefined): boolean {
|
||||
if (typeof ptyId !== 'string') {
|
||||
return false
|
||||
}
|
||||
return isRemoteRuntimePtyId(ptyId) || parseAppSshPtyId(ptyId) !== null
|
||||
}
|
||||
@@ -0,0 +1,192 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { mergeDirectSshRemoteWorkspaceSession } from './remote-workspace-session-merge'
|
||||
import { worktreeWorkspaceKey } from '../../../shared/workspace-scope'
|
||||
import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types'
|
||||
import type { TerminalTab } from '../../../shared/terminal-tab-types'
|
||||
import type { AppState } from '../store/types'
|
||||
|
||||
/**
|
||||
* The one thing a close tombstone is allowed to do, and everything it is not.
|
||||
*
|
||||
* The bug it exists for: the `pty.kill` for a closed SSH tab rejects with a transport error, so the
|
||||
* close never reaches the host, the host keeps listing the tab, and every reconnect re-inserts it.
|
||||
* The bug it must never cause is the opposite one — PR #14361 was reverted for deleting live tabs —
|
||||
* so every case below that is not "an id this client watched the user close" keeps the tab.
|
||||
*/
|
||||
const WORKTREE = 'repo-1::/home/user/bug-cats'
|
||||
|
||||
function terminalTab(id: string, overrides: Partial<TerminalTab> = {}): TerminalTab {
|
||||
return { id, title: id, type: 'terminal', worktreeId: WORKTREE, ...overrides } as TerminalTab
|
||||
}
|
||||
|
||||
function sessionState(overrides: Partial<WorkspaceSessionState> = {}): WorkspaceSessionState {
|
||||
return {
|
||||
activeRepoId: 'repo-1',
|
||||
activeWorktreeId: WORKTREE,
|
||||
activeWorkspaceKey: worktreeWorkspaceKey(WORKTREE),
|
||||
activeTabId: 'agent',
|
||||
tabsByWorktree: {},
|
||||
terminalLayoutsByTabId: {},
|
||||
activeTabIdByWorktree: {},
|
||||
...overrides
|
||||
} as WorkspaceSessionState
|
||||
}
|
||||
|
||||
function merge(
|
||||
current: WorkspaceSessionState,
|
||||
remote: WorkspaceSessionState,
|
||||
liveTabs: AppState['tabsByWorktree'] = {},
|
||||
remoteRevision?: number
|
||||
): WorkspaceSessionState {
|
||||
return mergeDirectSshRemoteWorkspaceSession(
|
||||
current,
|
||||
remote,
|
||||
new Set([WORKTREE]),
|
||||
liveTabs,
|
||||
new Set(),
|
||||
undefined,
|
||||
remoteRevision
|
||||
)
|
||||
}
|
||||
|
||||
function tombstone(tabId: string, ackRevision?: number): WorkspaceSessionState {
|
||||
return sessionState({
|
||||
tabsByWorktree: { [WORKTREE]: [] },
|
||||
closedTerminalTabTombstonesByTabId: {
|
||||
[tabId]: {
|
||||
closedAt: Date.now(),
|
||||
worktreeId: WORKTREE,
|
||||
...(ackRevision ? { ackRevision } : {})
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
describe('direct-SSH pull merge: closed-tab tombstones', () => {
|
||||
it('does not suppress a host tab whose id collides with an Object.prototype key', () => {
|
||||
// `tabId in map` answers true for every prototype key even on an EMPTY map, so a host tab named
|
||||
// `toString` was filtered out, blocked from the host-unknown branch, and stripped of its layout
|
||||
// and session id — the one path in this function that could delete a tab the user never closed.
|
||||
// Tab ids are validated only as non-empty and colon-free, and createTab honours caller-supplied
|
||||
// id hints, so the id is reachable rather than theoretical.
|
||||
const current = sessionState({ tabsByWorktree: { [WORKTREE]: [] } })
|
||||
const remote = sessionState({
|
||||
tabsByWorktree: { [WORKTREE]: [terminalTab('toString')] },
|
||||
terminalLayoutsByTabId: { toString: { root: { type: 'leaf', paneId: 'p' } } as never },
|
||||
remoteSessionIdsByTabId: { toString: 'session-1' }
|
||||
})
|
||||
|
||||
const merged = merge(current, remote, {}, 2)
|
||||
|
||||
expect(merged.tabsByWorktree[WORKTREE]?.map((tab) => tab.id)).toEqual(['toString'])
|
||||
expect(Object.hasOwn(merged.terminalLayoutsByTabId, 'toString')).toBe(true)
|
||||
expect(merged.remoteSessionIdsByTabId?.toString).toBe('session-1')
|
||||
})
|
||||
|
||||
it('does not suppress a tombstoned id under a DIFFERENT worktree', () => {
|
||||
// The tombstone carries the worktree it was closed in, so suppression cannot reach another
|
||||
// workspace's tab. Structural, rather than a property of where the call sites happen to sit.
|
||||
const other = 'repo-1::/home/user/other'
|
||||
const current = sessionState({
|
||||
tabsByWorktree: { [WORKTREE]: [] },
|
||||
closedTerminalTabTombstonesByTabId: {
|
||||
'tab-x': { closedAt: Date.now(), worktreeId: other }
|
||||
}
|
||||
})
|
||||
const remote = sessionState({ tabsByWorktree: { [WORKTREE]: [terminalTab('tab-x')] } })
|
||||
|
||||
const merged = merge(current, remote, {}, 2)
|
||||
|
||||
expect(merged.tabsByWorktree[WORKTREE]?.map((tab) => tab.id)).toEqual(['tab-x'])
|
||||
})
|
||||
|
||||
it('does not re-insert a tombstoned tab the host still lists', () => {
|
||||
const ghost = terminalTab('ghost')
|
||||
const remote = sessionState({
|
||||
tabsByWorktree: { [WORKTREE]: [ghost] },
|
||||
terminalLayoutsByTabId: { ghost: { type: 'single', tabId: 'ghost' } as never },
|
||||
remoteSessionIdsByTabId: { ghost: 'pty-1' }
|
||||
})
|
||||
|
||||
const merged = merge(tombstone('ghost'), remote, { [WORKTREE]: [] }, 3)
|
||||
|
||||
expect(merged.tabsByWorktree[WORKTREE]).toEqual([])
|
||||
expect(merged.terminalLayoutsByTabId.ghost).toBeUndefined()
|
||||
expect(merged.remoteSessionIdsByTabId?.ghost).toBeUndefined()
|
||||
expect(merged.closedTerminalTabTombstonesByTabId?.ghost).toBeDefined()
|
||||
})
|
||||
|
||||
it('does not re-add a tombstoned tab through the host-unknown branch', () => {
|
||||
// The stale persisted payload still lists it, and the host has never heard of it. Without the
|
||||
// tombstone that combination is exactly what the host-unknown branch exists to preserve.
|
||||
const ghost = terminalTab('ghost')
|
||||
const current = sessionState({
|
||||
tabsByWorktree: { [WORKTREE]: [ghost] },
|
||||
closedTerminalTabTombstonesByTabId: { ghost: { closedAt: Date.now(), worktreeId: WORKTREE } }
|
||||
})
|
||||
const remote = sessionState({ tabsByWorktree: { [WORKTREE]: [] } })
|
||||
|
||||
const merged = merge(current, remote, {}, 3)
|
||||
|
||||
expect(merged.tabsByWorktree[WORKTREE]).toEqual([])
|
||||
})
|
||||
|
||||
it('suppresses only the tombstoned tab, leaving its siblings and the pointers alone', () => {
|
||||
// The active-tab pointers deliberately still name the ghost: hydration validates both of them
|
||||
// against the rows below and nulls what they no longer name, so the merge leaves them be.
|
||||
const ghost = terminalTab('ghost')
|
||||
const survivor = terminalTab('survivor')
|
||||
const remote = sessionState({
|
||||
tabsByWorktree: { [WORKTREE]: [ghost, survivor] },
|
||||
activeTabId: 'ghost',
|
||||
activeTabIdByWorktree: { [WORKTREE]: 'ghost' }
|
||||
})
|
||||
|
||||
const merged = merge(tombstone('ghost'), remote, { [WORKTREE]: [] }, 3)
|
||||
|
||||
expect(merged.tabsByWorktree[WORKTREE].map((tab) => tab.id)).toEqual(['survivor'])
|
||||
})
|
||||
|
||||
it('a live local tab beats a stale tombstone for its id', () => {
|
||||
const live = terminalTab('live')
|
||||
const current = sessionState({
|
||||
tabsByWorktree: { [WORKTREE]: [live] },
|
||||
closedTerminalTabTombstonesByTabId: { live: { closedAt: Date.now(), worktreeId: WORKTREE } }
|
||||
})
|
||||
const remote = sessionState({ tabsByWorktree: { [WORKTREE]: [live] } })
|
||||
|
||||
const merged = merge(current, remote, { [WORKTREE]: [live] }, 3)
|
||||
|
||||
expect(merged.tabsByWorktree[WORKTREE].map((tab) => tab.id)).toEqual(['live'])
|
||||
})
|
||||
|
||||
it('leaves a tab alone when no tombstone names it', () => {
|
||||
// The pre-existing absence-is-not-closed trade, restated: a host that still lists a tab closed
|
||||
// on some other client keeps it here, because absence is all this merge would have to go on.
|
||||
const agent = terminalTab('agent')
|
||||
const closedElsewhere = terminalTab('closed-elsewhere')
|
||||
const current = sessionState({ tabsByWorktree: { [WORKTREE]: [agent, closedElsewhere] } })
|
||||
const remote = sessionState({ tabsByWorktree: { [WORKTREE]: [agent] } })
|
||||
|
||||
const merged = merge(current, remote, { [WORKTREE]: [agent, closedElsewhere] }, 3)
|
||||
|
||||
expect(merged.tabsByWorktree[WORKTREE].map((tab) => tab.id)).toContain('closed-elsewhere')
|
||||
})
|
||||
|
||||
it('retires the tombstone once a newer snapshot stops listing the tab', () => {
|
||||
const remote = sessionState({ tabsByWorktree: { [WORKTREE]: [] } })
|
||||
|
||||
const merged = merge(tombstone('ghost', 3), remote, { [WORKTREE]: [] }, 4)
|
||||
|
||||
expect(merged.closedTerminalTabTombstonesByTabId).toBeUndefined()
|
||||
})
|
||||
|
||||
it('keeps the tombstone when the snapshot omits the worktree entirely', () => {
|
||||
// A snapshot that says nothing about the worktree — including one whose path never resolved —
|
||||
// is not evidence the host saw the close.
|
||||
const merged = merge(tombstone('ghost', 3), sessionState(), {}, 4)
|
||||
|
||||
expect(merged.closedTerminalTabTombstonesByTabId?.ghost).toBeDefined()
|
||||
})
|
||||
})
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { TerminalTab } from '../../../shared/terminal-tab-types'
|
||||
import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types'
|
||||
import { reconcileClosedTerminalTabTombstones } from '../../../shared/closed-terminal-tab-tombstones'
|
||||
import type { ExecutionHostId } from '../../../shared/execution-host'
|
||||
import { worktreeWorkspaceKey } from '../../../shared/workspace-scope'
|
||||
import { splitWorktreeId } from '../../../shared/worktree/id'
|
||||
@@ -26,8 +27,21 @@ export function mergeDirectSshRemoteWorkspaceSession(
|
||||
replaceWorktreeIds: ReadonlySet<string>,
|
||||
liveTabsByWorktree: AppState['tabsByWorktree'],
|
||||
preserveLocalTerminalTabIds: ReadonlySet<string>,
|
||||
replaceExecutionHostId?: ExecutionHostId
|
||||
replaceExecutionHostId?: ExecutionHostId,
|
||||
remoteRevision?: number
|
||||
): WorkspaceSessionState {
|
||||
// Live tabs across the worktrees this snapshot replaces. Close-suppression consults it so a tab
|
||||
// that is still live locally always beats its own tombstone.
|
||||
// What actually keeps suppression from deleting a tab the user did not close — the earlier claim
|
||||
// that "every use is inside replaceWorktreeIds" was wrong, since the tabsByWorktree pass below
|
||||
// walks all of orderedWorktreeIds and the layout/session sweeps cover the whole remote maps:
|
||||
// 1. closeTab strips the id from EVERY worktree row before recording the tombstone
|
||||
// (terminal-tab-close.ts), so a tombstoned id is not live anywhere;
|
||||
// 2. isSuppressedByClose matches the tombstone's own worktreeId, so it cannot reach another
|
||||
// workspace's tab even if an id somehow recurred;
|
||||
// 3. only closeReason === 'user' ever writes a tombstone.
|
||||
// A final sweep of suppression over the assembled tabsByWorktree — including worktrees
|
||||
// omitTargetWorktrees passes through verbatim — is still deliberately absent.
|
||||
const currentTabsById = new Map(
|
||||
[...replaceWorktreeIds]
|
||||
.flatMap((worktreeId) => liveTabsByWorktree[worktreeId] ?? [])
|
||||
@@ -96,22 +110,64 @@ export function mergeDirectSshRemoteWorkspaceSession(
|
||||
.filter(([tabId]) => remoteKnownTabIds.has(tabId))
|
||||
.map(([, sessionId]) => sessionId)
|
||||
)
|
||||
// The other half of the trade below. Absence still cannot say "closed", so this says it instead:
|
||||
// a tab THIS client watched the user close, whose close never reached the host. Only ids the user
|
||||
// closed are ever in here, and only until the host's own snapshot stops listing them.
|
||||
const closedTerminalTabTombstonesByTabId = reconcileClosedTerminalTabTombstones({
|
||||
tombstones: current.closedTerminalTabTombstonesByTabId,
|
||||
acknowledgedWorktreeIds: new Set(
|
||||
[...replaceWorktreeIds].filter((worktreeId) =>
|
||||
Object.hasOwn(remote.tabsByWorktree, worktreeId)
|
||||
)
|
||||
),
|
||||
hostKnownTabIds: remoteKnownTabIds,
|
||||
hostRevision: remoteRevision,
|
||||
now: Date.now()
|
||||
})
|
||||
// Why Object.hasOwn and not `in`: the map is a plain object from Object.fromEntries, so `in`
|
||||
// answers true for every Object.prototype key on an EMPTY map — a host tab whose id is
|
||||
// `toString` would be filtered, blocked from the host-unknown branch, and stripped of its layout
|
||||
// and session id. Tab ids are validated only as non-empty and colon-free, and createTab honours
|
||||
// caller-supplied id hints, so that id is reachable. This is the one place either direction could
|
||||
// delete a tab the user never closed.
|
||||
// Why the worktree comparison: the tombstone already carries the worktree it was closed in, so
|
||||
// scoping on it makes "this suppression cannot reach another workspace's tab" structural rather
|
||||
// than a property of where the call sites happen to sit.
|
||||
// Why a live local tab still overrides its own tombstone: an id that is live here means the
|
||||
// tombstone is stale (a close undone before it persisted), not a revival. Deleting a live pane is
|
||||
// the one outcome this whole function exists to avoid.
|
||||
const isSuppressedByClose = (tabId: string, worktreeId: string): boolean =>
|
||||
Object.hasOwn(closedTerminalTabTombstonesByTabId, tabId) &&
|
||||
closedTerminalTabTombstonesByTabId[tabId]?.worktreeId === worktreeId &&
|
||||
!currentTabsById.has(tabId)
|
||||
// Ids this merge actually suppressed, recorded as it walks the worktrees. The layout and
|
||||
// session-id sweeps below have no worktree in scope, so they consult decisions already made
|
||||
// rather than re-deriving one without the scope that makes it safe.
|
||||
const suppressedTabIds = new Set<string>()
|
||||
const tabsByWorktree = Object.fromEntries(
|
||||
orderedWorktreeIds.map((worktreeId) => {
|
||||
const remoteTabs = remote.tabsByWorktree[worktreeId] ?? []
|
||||
const reconciled = remoteTabs.map((tab) => {
|
||||
const local = currentTabsById.get(tab.id)
|
||||
if (
|
||||
!local ||
|
||||
((local.generation ?? 0) <= (tab.generation ?? 0) &&
|
||||
!local.pendingActivationSpawn &&
|
||||
!preserveLocalTerminalTabIds.has(tab.id))
|
||||
) {
|
||||
return tab
|
||||
}
|
||||
locallyPreservedTabIds.add(tab.id)
|
||||
return preserveNewerLocalTerminalFields(tab, local)
|
||||
})
|
||||
const reconciled = remoteTabs
|
||||
.filter((tab) => {
|
||||
if (!isSuppressedByClose(tab.id, worktreeId)) {
|
||||
return true
|
||||
}
|
||||
suppressedTabIds.add(tab.id)
|
||||
return false
|
||||
})
|
||||
.map((tab) => {
|
||||
const local = currentTabsById.get(tab.id)
|
||||
if (
|
||||
!local ||
|
||||
((local.generation ?? 0) <= (tab.generation ?? 0) &&
|
||||
!local.pendingActivationSpawn &&
|
||||
!preserveLocalTerminalTabIds.has(tab.id))
|
||||
) {
|
||||
return tab
|
||||
}
|
||||
locallyPreservedTabIds.add(tab.id)
|
||||
return preserveNewerLocalTerminalFields(tab, local)
|
||||
})
|
||||
for (const tab of reconciled) {
|
||||
emittedTabIds.add(tab.id)
|
||||
}
|
||||
@@ -135,6 +191,10 @@ export function mergeDirectSshRemoteWorkspaceSession(
|
||||
if (remoteKnownTabIds.has(tab.id) || emittedTabIds.has(tab.id)) {
|
||||
return false
|
||||
}
|
||||
if (isSuppressedByClose(tab.id, worktreeId)) {
|
||||
suppressedTabIds.add(tab.id)
|
||||
return false
|
||||
}
|
||||
const localSessionId = current.remoteSessionIdsByTabId?.[tab.id]
|
||||
return localSessionId == null || !remoteKnownSessionIds.has(localSessionId)
|
||||
})
|
||||
@@ -186,7 +246,7 @@ export function mergeDirectSshRemoteWorkspaceSession(
|
||||
),
|
||||
...Object.fromEntries(
|
||||
Object.entries(remote.terminalLayoutsByTabId).filter(
|
||||
([tabId]) => !locallyPreservedTabIds.has(tabId)
|
||||
([tabId]) => !locallyPreservedTabIds.has(tabId) && !suppressedTabIds.has(tabId)
|
||||
)
|
||||
)
|
||||
}
|
||||
@@ -243,6 +303,9 @@ export function mergeDirectSshRemoteWorkspaceSession(
|
||||
return localActiveTabId == null ? [] : [[worktreeId, localActiveTabId] as const]
|
||||
})
|
||||
),
|
||||
// Why a suppressed id is left in place here: hydration validates both active-tab pointers
|
||||
// against the tab rows it just built and nulls anything they no longer name, so nulling twice
|
||||
// would only add a second rule that has to stay in step with that one.
|
||||
...remote.activeTabIdByWorktree
|
||||
},
|
||||
remoteSessionIdsByTabId: {
|
||||
@@ -253,7 +316,7 @@ export function mergeDirectSshRemoteWorkspaceSession(
|
||||
),
|
||||
...Object.fromEntries(
|
||||
Object.entries(remote.remoteSessionIdsByTabId ?? {}).filter(
|
||||
([tabId]) => !locallyPreservedTabIds.has(tabId)
|
||||
([tabId]) => !locallyPreservedTabIds.has(tabId) && !suppressedTabIds.has(tabId)
|
||||
)
|
||||
)
|
||||
},
|
||||
@@ -268,7 +331,11 @@ export function mergeDirectSshRemoteWorkspaceSession(
|
||||
// tabs. Removal is the worktree-teardown path's job, not a reconnect's.
|
||||
...current.defaultTerminalTabsAppliedByWorktreeId,
|
||||
...remote.defaultTerminalTabsAppliedByWorktreeId
|
||||
}
|
||||
},
|
||||
closedTerminalTabTombstonesByTabId:
|
||||
Object.keys(closedTerminalTabTombstonesByTabId).length > 0
|
||||
? closedTerminalTabTombstonesByTabId
|
||||
: undefined
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -119,7 +119,8 @@ export async function applyDirectSshRemoteWorkspaceSnapshot({
|
||||
worktreeIds,
|
||||
state.tabsByWorktree,
|
||||
currentRecoveryTabIds(state, authority, worktreeIds),
|
||||
toSshExecutionHostId(authority.targetId)
|
||||
toSshExecutionHostId(authority.targetId),
|
||||
snapshot.revision
|
||||
)
|
||||
if (!isArrivalCurrent(authority.targetId, arrival) || !isPreparationTokenCurrent(token)) {
|
||||
return
|
||||
|
||||
@@ -32,7 +32,8 @@ function createSnapshot(browserUrlHistory: BrowserHistoryEntry[]): WorkspaceSess
|
||||
worktreesByRepo: {},
|
||||
lastKnownRelayPtyIdByTabId: {},
|
||||
lastVisitedAtByWorktreeId: {},
|
||||
defaultTerminalTabsAppliedByWorktreeId: {}
|
||||
defaultTerminalTabsAppliedByWorktreeId: {},
|
||||
closedTerminalTabTombstonesByTabId: {}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,38 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { buildWorkspaceSessionPatch } from './workspace-session-patch'
|
||||
import { SESSION_RELEVANT_FIELDS } from './workspace-session'
|
||||
import { CLOSED_TERMINAL_TAB_TOMBSTONE_TTL_MS } from '../../../shared/closed-terminal-tab-tombstones'
|
||||
|
||||
// Why browserPagesByWorkspace is stubbed: buildWorkspaceSessionPatch pre-filters staged browser tabs
|
||||
// before field dispatch, so the minimal fixture has to carry the map it iterates.
|
||||
function patchFor(
|
||||
map: Record<string, { closedAt: number; worktreeId: string }>
|
||||
): ReturnType<typeof buildWorkspaceSessionPatch> {
|
||||
return buildWorkspaceSessionPatch(
|
||||
{ browserPagesByWorkspace: {}, closedTerminalTabTombstonesByTabId: map } as never,
|
||||
['closedTerminalTabTombstonesByTabId']
|
||||
)
|
||||
}
|
||||
|
||||
describe('closed-tab tombstones in the persisted session', () => {
|
||||
it('is a session-relevant field, so a close on its own schedules a write', () => {
|
||||
expect(SESSION_RELEVANT_FIELDS).toContain('closedTerminalTabTombstonesByTabId')
|
||||
})
|
||||
|
||||
it('writes live tombstones and drops ones past the TTL', () => {
|
||||
const now = Date.now()
|
||||
expect(
|
||||
patchFor({
|
||||
fresh: { closedAt: now, worktreeId: 'repo-1::/srv/app' },
|
||||
stale: {
|
||||
closedAt: now - CLOSED_TERMINAL_TAB_TOMBSTONE_TTL_MS - 1,
|
||||
worktreeId: 'repo-1::/srv/app'
|
||||
}
|
||||
}).closedTerminalTabTombstonesByTabId
|
||||
).toEqual({ fresh: { closedAt: now, worktreeId: 'repo-1::/srv/app' } })
|
||||
})
|
||||
|
||||
it('omits the field entirely once the map empties', () => {
|
||||
expect(patchFor({}).closedTerminalTabTombstonesByTabId).toBeUndefined()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,10 @@
|
||||
import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types'
|
||||
import { pruneClosedTerminalTabTombstones } from '../../../shared/closed-terminal-tab-tombstones'
|
||||
|
||||
/** Applies the TTL/cap bound on the way to disk, and omits an empty map like its siblings. */
|
||||
export function buildPersistedClosedTerminalTabTombstones(
|
||||
map: WorkspaceSessionState['closedTerminalTabTombstonesByTabId']
|
||||
): WorkspaceSessionState['closedTerminalTabTombstonesByTabId'] {
|
||||
const pruned = pruneClosedTerminalTabTombstones(map, Date.now())
|
||||
return Object.keys(pruned).length > 0 ? pruned : undefined
|
||||
}
|
||||
@@ -34,6 +34,7 @@ function createSnapshot(
|
||||
lastKnownRelayPtyIdByTabId: {},
|
||||
lastVisitedAtByWorktreeId: {},
|
||||
defaultTerminalTabsAppliedByWorktreeId: {},
|
||||
closedTerminalTabTombstonesByTabId: {},
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
@@ -49,7 +49,10 @@ export const WORKSPACE_SESSION_FIELD_OWNERSHIP = {
|
||||
terminalPtyIncarnationsByPaneKey: 'paneKeyed',
|
||||
// Why: this host-issued fence must never collide while unified renderer state merges equal repo ids across hosts.
|
||||
terminalTopologyRevisionByRepoId: 'hostPrivate',
|
||||
terminalSurfaceTombstonesByPaneKey: 'surfaceTombstoneKeyed'
|
||||
terminalSurfaceTombstonesByPaneKey: 'surfaceTombstoneKeyed',
|
||||
// Why not tabKeyed: the tab is already gone, so worktreeIdByTabId can never resolve it. Routing by
|
||||
// the record's own worktreeId is the same problem terminalSurfaceTombstonesByPaneKey has.
|
||||
closedTerminalTabTombstonesByTabId: 'surfaceTombstoneKeyed'
|
||||
} as const satisfies Record<keyof WorkspaceSessionState, WorkspaceSessionFieldOwnership>
|
||||
|
||||
// Why: an unclassified persisted field would otherwise disappear from every non-local host.
|
||||
|
||||
@@ -49,7 +49,9 @@ export type HostIdByWorktreeId = (worktreeId: string) => ExecutionHostId
|
||||
* - browserWorkspaceKeyed: Record keyed by browser-workspace id; follows the
|
||||
* page record's own worktreeId.
|
||||
* - fileKeyed: Record keyed by editor file id; follows the open file's worktree.
|
||||
* - sleepingAgentKeyed: Record keyed by pane key; follows the record's worktreeId. */
|
||||
* - sleepingAgentKeyed: Record keyed by pane key; follows the record's worktreeId.
|
||||
* - surfaceTombstoneKeyed: Record under an opaque key whose value names its own worktreeId --
|
||||
* the only routing available once the tab or pane it describes is gone. */
|
||||
type SplitContext = {
|
||||
hostIdByWorktreeId: HostIdByWorktreeId
|
||||
worktreeIdByTabId: Map<string, string>
|
||||
|
||||
@@ -33,6 +33,7 @@ function createSnapshot(
|
||||
lastKnownRelayPtyIdByTabId: {},
|
||||
lastVisitedAtByWorktreeId: {},
|
||||
defaultTerminalTabsAppliedByWorktreeId: {},
|
||||
closedTerminalTabTombstonesByTabId: {},
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,6 +19,7 @@ import { withoutStagedBrowserTabs } from './workspace-session-staged-browser-tab
|
||||
import { buildPersistedUnifiedTabSessionData } from './workspace-session-unified-tabs'
|
||||
import { buildLastVisitedAtByWorktreeId } from './workspace-session-focus-recency'
|
||||
import { buildSleepingAgentSessionData } from './workspace-session-sleeping-agents'
|
||||
import { buildPersistedClosedTerminalTabTombstones } from './workspace-session-closed-tab-tombstones'
|
||||
|
||||
type SessionRelevantField = keyof WorkspaceSessionSnapshot
|
||||
|
||||
@@ -160,6 +161,11 @@ export function buildWorkspaceSessionPatch(
|
||||
? snapshot.defaultTerminalTabsAppliedByWorktreeId
|
||||
: undefined
|
||||
}
|
||||
if (changed.has('closedTerminalTabTombstonesByTabId')) {
|
||||
patch.closedTerminalTabTombstonesByTabId = buildPersistedClosedTerminalTabTombstones(
|
||||
snapshot.closedTerminalTabTombstonesByTabId
|
||||
)
|
||||
}
|
||||
if (changed.has('sleepingAgentSessionsByPaneKey')) {
|
||||
patch.sleepingAgentSessionsByPaneKey =
|
||||
buildSleepingAgentSessionData(snapshot).sleepingAgentSessionsByPaneKey
|
||||
|
||||
@@ -34,6 +34,7 @@ describe('SESSION_RELEVANT_FIELDS', () => {
|
||||
lastKnownRelayPtyIdByTabId: true,
|
||||
lastVisitedAtByWorktreeId: true,
|
||||
defaultTerminalTabsAppliedByWorktreeId: true,
|
||||
closedTerminalTabTombstonesByTabId: true,
|
||||
sleepingAgentSessionsByPaneKey: true,
|
||||
clientHostedBrowserCloseIntentsByEnvironment: true
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@ import type { OpenFile } from '../store/slices/editor'
|
||||
import { buildPersistedUnifiedTabSessionData } from './workspace-session-unified-tabs'
|
||||
import { buildLastVisitedAtByWorktreeId } from './workspace-session-focus-recency'
|
||||
import { buildSleepingAgentSessionData } from './workspace-session-sleeping-agents'
|
||||
import { buildPersistedClosedTerminalTabTombstones } from './workspace-session-closed-tab-tombstones'
|
||||
import { buildActiveConnectionIdsAtShutdown } from './workspace-session-reconnect-targets'
|
||||
import { withoutStagedBrowserTabs } from './workspace-session-staged-browser-tabs'
|
||||
import { buildBrowserSessionData } from './workspace-session-browser-tabs'
|
||||
@@ -53,6 +54,7 @@ export type WorkspaceSessionSnapshot = Pick<
|
||||
| 'lastKnownRelayPtyIdByTabId'
|
||||
| 'lastVisitedAtByWorktreeId'
|
||||
| 'defaultTerminalTabsAppliedByWorktreeId'
|
||||
| 'closedTerminalTabTombstonesByTabId'
|
||||
> & {
|
||||
activeWorkspaceExecutionHostId?: AppState['activeWorkspaceExecutionHostId']
|
||||
sleepingAgentSessionsByPaneKey?: AppState['sleepingAgentSessionsByPaneKey']
|
||||
@@ -90,6 +92,7 @@ export const SESSION_RELEVANT_FIELDS = [
|
||||
'lastKnownRelayPtyIdByTabId',
|
||||
'lastVisitedAtByWorktreeId',
|
||||
'defaultTerminalTabsAppliedByWorktreeId',
|
||||
'closedTerminalTabTombstonesByTabId',
|
||||
'sleepingAgentSessionsByPaneKey',
|
||||
'clientHostedBrowserCloseIntentsByEnvironment'
|
||||
] as const satisfies readonly (keyof WorkspaceSessionSnapshot)[]
|
||||
@@ -303,6 +306,9 @@ export function buildWorkspaceSessionPayload(
|
||||
Object.keys(snapshot.defaultTerminalTabsAppliedByWorktreeId).length > 0
|
||||
? snapshot.defaultTerminalTabsAppliedByWorktreeId
|
||||
: undefined,
|
||||
closedTerminalTabTombstonesByTabId: buildPersistedClosedTerminalTabTombstones(
|
||||
snapshot.closedTerminalTabTombstonesByTabId
|
||||
),
|
||||
...buildSleepingAgentSessionData(snapshot),
|
||||
// Why unconditional rather than omit-when-empty: a full write replaces the persisted object,
|
||||
// so an emptied map has to be written as empty or the last replay never sticks.
|
||||
|
||||
@@ -49,6 +49,7 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
|
||||
workspaceSessionReady: false,
|
||||
restoredRuntimeHostIdByWorkspaceSessionKey: {},
|
||||
defaultTerminalTabsAppliedByWorktreeId: {},
|
||||
closedTerminalTabTombstonesByTabId: {},
|
||||
hydrationSucceeded: false,
|
||||
pendingReconnectWorktreeIds: [],
|
||||
pendingReconnectTabByWorktree: {},
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import type { ClosedTerminalTabTombstonesByTabId } from '../../../../shared/closed-terminal-tab-tombstones'
|
||||
import type { TerminalLayoutSnapshot, TerminalTab } from '../../../../shared/terminal-tab-types'
|
||||
import type { TuiAgent } from '../../../../shared/tui-agent'
|
||||
import type { SetupSplitDirection } from '../../../../shared/worktree/launch-types'
|
||||
@@ -90,6 +91,7 @@ export type TerminalState = {
|
||||
workspaceSessionReady: boolean
|
||||
restoredRuntimeHostIdByWorkspaceSessionKey: Record<string, ExecutionHostId>
|
||||
defaultTerminalTabsAppliedByWorktreeId: Record<string, true>
|
||||
closedTerminalTabTombstonesByTabId: ClosedTerminalTabTombstonesByTabId
|
||||
hydrationSucceeded: boolean
|
||||
pendingReconnectWorktreeIds: string[]
|
||||
pendingReconnectTabByWorktree: Record<string, string[]>
|
||||
|
||||
@@ -0,0 +1,80 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import type * as AgentStatusModule from '@/lib/agent-status'
|
||||
import { createTestStore, makeTab, makeWorktree, seedStore } from '../slices/store-test-helpers'
|
||||
import { createStoreCascadesMockApi } from '../slices/store-cascades-test-harness'
|
||||
|
||||
vi.mock('sonner', () => ({
|
||||
toast: { info: vi.fn(), success: vi.fn(), error: vi.fn(), warning: vi.fn() }
|
||||
}))
|
||||
|
||||
vi.mock('@/components/terminal-pane/pty-dispatcher', () => ({
|
||||
restorePtyDataHandlersAfterFailedShutdown: vi.fn(),
|
||||
unregisterPtyDataHandlers: vi.fn<() => unknown[]>(() => [])
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/agent-status', async (importOriginal) => ({
|
||||
...(await importOriginal<typeof AgentStatusModule>()),
|
||||
detectAgentStatusFromTitle: vi.fn().mockReturnValue(null)
|
||||
}))
|
||||
|
||||
const mockApi = createStoreCascadesMockApi()
|
||||
|
||||
const REMOTE_WORKTREE = 'remote-repo::/srv/app'
|
||||
const LOCAL_WORKTREE = 'local-repo::/tmp/app'
|
||||
|
||||
function storeWithBothWorktrees(): ReturnType<typeof createTestStore> {
|
||||
const store = createTestStore()
|
||||
seedStore(store, {
|
||||
repos: [
|
||||
{ id: 'remote-repo', path: '/srv/app', name: 'app', connectionId: 'ssh-1' },
|
||||
{ id: 'local-repo', path: '/tmp/app', name: 'app' }
|
||||
] as never,
|
||||
worktreesByRepo: {
|
||||
'remote-repo': [
|
||||
makeWorktree({ id: REMOTE_WORKTREE, repoId: 'remote-repo', path: '/srv/app' })
|
||||
],
|
||||
'local-repo': [makeWorktree({ id: LOCAL_WORKTREE, repoId: 'local-repo', path: '/tmp/app' })]
|
||||
},
|
||||
tabsByWorktree: {
|
||||
[REMOTE_WORKTREE]: [makeTab({ id: 'remote-tab', worktreeId: REMOTE_WORKTREE })],
|
||||
[LOCAL_WORKTREE]: [makeTab({ id: 'local-tab', worktreeId: LOCAL_WORKTREE })]
|
||||
}
|
||||
})
|
||||
return store
|
||||
}
|
||||
|
||||
describe('closeTab close tombstones', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockApi.worktrees.updateMeta.mockResolvedValue({})
|
||||
})
|
||||
|
||||
it('records the closed tab against the worktree it was closed in', () => {
|
||||
const store = storeWithBothWorktrees()
|
||||
|
||||
store.getState().closeTab('remote-tab')
|
||||
|
||||
expect(store.getState().closedTerminalTabTombstonesByTabId['remote-tab']).toEqual({
|
||||
closedAt: expect.any(Number),
|
||||
worktreeId: REMOTE_WORKTREE
|
||||
})
|
||||
})
|
||||
|
||||
// A tombstone outlives the host's own record, so the only claim it may ever make is "the user
|
||||
// closed this". Neither of these closes is that claim.
|
||||
it.each([['pty-exit'], ['cleanup']] as const)('records nothing for a %s close', (reason) => {
|
||||
const store = storeWithBothWorktrees()
|
||||
|
||||
store.getState().closeTab('remote-tab', { reason })
|
||||
|
||||
expect(store.getState().closedTerminalTabTombstonesByTabId['remote-tab']).toBeUndefined()
|
||||
})
|
||||
|
||||
it('records nothing for a definitively local worktree, which no pull merge ever reads', () => {
|
||||
const store = storeWithBothWorktrees()
|
||||
|
||||
store.getState().closeTab('local-tab')
|
||||
|
||||
expect(store.getState().closedTerminalTabTombstonesByTabId).toEqual({})
|
||||
})
|
||||
})
|
||||
@@ -1,4 +1,6 @@
|
||||
import { recordClosedTerminalTabTombstone } from '../../../../shared/closed-terminal-tab-tombstones'
|
||||
import type { TerminalTab } from '../../../../shared/terminal-tab-types'
|
||||
import { getConnectionIdFromState } from '@/lib/connection-owner-resolution'
|
||||
import { sweepRetiredTerminalTabState } from '../slices/retired-terminal-tab-state-sweep'
|
||||
import {
|
||||
getRecentlyClosedTabPosition,
|
||||
@@ -58,6 +60,23 @@ export function createTerminalTabCloseActions(
|
||||
next[wId] = after
|
||||
}
|
||||
}
|
||||
// Why `user` and not retiresSession: a tombstone outlives the host's own record, so the only
|
||||
// thing it may ever say is "the user closed this". A pty-exit close is the process ending,
|
||||
// and a `cleanup` close retires a tab the app itself created — neither is that claim.
|
||||
// Why a non-local worktree only: the tombstone is read solely by the direct-SSH pull merge,
|
||||
// and a definitively local tab would just consume the map's cap. An unresolved repo
|
||||
// (undefined, not null) still records — losing the tombstone reinstates the resurrection.
|
||||
const nextClosedTombstones =
|
||||
closeReason === 'user' &&
|
||||
closedWorktreeId &&
|
||||
getConnectionIdFromState(s, closedWorktreeId) !== null
|
||||
? recordClosedTerminalTabTombstone(
|
||||
s.closedTerminalTabTombstonesByTabId,
|
||||
tabId,
|
||||
closedWorktreeId,
|
||||
Date.now()
|
||||
)
|
||||
: s.closedTerminalTabTombstonesByTabId
|
||||
// Why: only explicit user closes feed the Cmd+Shift+T reopen stack; cleanup/PTY-exit closes must not pollute undo history.
|
||||
const closedPosition =
|
||||
closedWorktreeId && closedTab
|
||||
@@ -207,6 +226,9 @@ export function createTerminalTabCloseActions(
|
||||
...(nextSleepingAgentSessionsByPaneKey !== s.sleepingAgentSessionsByPaneKey
|
||||
? { sleepingAgentSessionsByPaneKey: nextSleepingAgentSessionsByPaneKey }
|
||||
: {}),
|
||||
...(nextClosedTombstones !== s.closedTerminalTabTombstonesByTabId
|
||||
? { closedTerminalTabTombstonesByTabId: nextClosedTombstones }
|
||||
: {}),
|
||||
// Why: skip writing unreadTerminalTabs when unchanged to avoid a no-op state allocation that re-evaluates full-state selectors. Mirrors tabs.ts.
|
||||
...(nextUnreadTerminalTabs !== s.unreadTerminalTabs
|
||||
? { unreadTerminalTabs: nextUnreadTerminalTabs }
|
||||
|
||||
@@ -19,6 +19,7 @@ export type WorkspaceHydrationPatch = Pick<
|
||||
| 'worktreesByRepo'
|
||||
| 'lastVisitedAtByWorktreeId'
|
||||
| 'defaultTerminalTabsAppliedByWorktreeId'
|
||||
| 'closedTerminalTabTombstonesByTabId'
|
||||
| 'automaticAgentResumeClaimsByTabId'
|
||||
| 'sleepingAgentSessionsByPaneKey'
|
||||
| 'pendingReconnectWorktreeIds'
|
||||
@@ -176,6 +177,9 @@ export function targetScopedWorkspaceHydrationPatch(
|
||||
hydrated.defaultTerminalTabsAppliedByWorktreeId,
|
||||
workspaceKeys
|
||||
),
|
||||
// Why passed through whole: hydration already unioned it with live store state, and the map is
|
||||
// keyed by tab id rather than by workspace key so replaceHydratedRecordKeys has nothing to match.
|
||||
closedTerminalTabTombstonesByTabId: hydrated.closedTerminalTabTombstonesByTabId,
|
||||
automaticAgentResumeClaimsByTabId: replaceHydratedRecordKeys(
|
||||
state.automaticAgentResumeClaimsByTabId,
|
||||
hydrated.automaticAgentResumeClaimsByTabId,
|
||||
|
||||
@@ -178,6 +178,11 @@ export function createWorkspaceTerminalHydrationActions(
|
||||
lastVisitedAtByWorktreeId: session.lastVisitedAtByWorktreeId ?? {},
|
||||
defaultTerminalTabsAppliedByWorktreeId:
|
||||
session.defaultTerminalTabsAppliedByWorktreeId ?? {},
|
||||
// Why replace and not union: both callers hand over a map they derived from this store
|
||||
// synchronously (the pull merge) or from disk before the store had one (startup), so there
|
||||
// is no local tombstone to lose — and a union would resurrect the ones the merge just
|
||||
// retired on the host's acknowledgement, which is the whole bound on this map.
|
||||
closedTerminalTabTombstonesByTabId: session.closedTerminalTabTombstonesByTabId ?? {},
|
||||
automaticAgentResumeClaimsByTabId: {},
|
||||
sleepingAgentSessionsByPaneKey,
|
||||
pendingReconnectWorktreeIds,
|
||||
|
||||
@@ -0,0 +1,109 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
CLOSED_TERMINAL_TAB_TOMBSTONE_TTL_MS,
|
||||
MAX_CLOSED_TERMINAL_TAB_TOMBSTONES,
|
||||
pruneClosedTerminalTabTombstones,
|
||||
recordClosedTerminalTabTombstone,
|
||||
reconcileClosedTerminalTabTombstones,
|
||||
type ClosedTerminalTabTombstonesByTabId
|
||||
} from './closed-terminal-tab-tombstones'
|
||||
|
||||
const NOW = 1_800_000_000_000
|
||||
const WT = 'repo-1::/srv/app'
|
||||
|
||||
function reconcile(
|
||||
tombstones: ClosedTerminalTabTombstonesByTabId,
|
||||
args: {
|
||||
acknowledgedWorktreeIds?: string[]
|
||||
hostKnownTabIds?: string[]
|
||||
hostRevision?: number
|
||||
}
|
||||
): ClosedTerminalTabTombstonesByTabId {
|
||||
return reconcileClosedTerminalTabTombstones({
|
||||
tombstones,
|
||||
acknowledgedWorktreeIds: new Set(args.acknowledgedWorktreeIds ?? [WT]),
|
||||
hostKnownTabIds: new Set(args.hostKnownTabIds ?? []),
|
||||
hostRevision: args.hostRevision,
|
||||
now: NOW
|
||||
})
|
||||
}
|
||||
|
||||
describe('closed terminal tab tombstones', () => {
|
||||
it('records the closing worktree and time', () => {
|
||||
expect(recordClosedTerminalTabTombstone({}, 'tab-1', WT, NOW)).toEqual({
|
||||
'tab-1': { closedAt: NOW, worktreeId: WT }
|
||||
})
|
||||
})
|
||||
|
||||
it('prunes past the TTL and caps at the newest entries', () => {
|
||||
expect(
|
||||
pruneClosedTerminalTabTombstones(
|
||||
{
|
||||
fresh: { closedAt: NOW - 1_000, worktreeId: WT },
|
||||
stale: { closedAt: NOW - CLOSED_TERMINAL_TAB_TOMBSTONE_TTL_MS - 1, worktreeId: WT }
|
||||
},
|
||||
NOW
|
||||
)
|
||||
).toEqual({ fresh: { closedAt: NOW - 1_000, worktreeId: WT } })
|
||||
|
||||
const overflowing = Object.fromEntries(
|
||||
Array.from({ length: MAX_CLOSED_TERMINAL_TAB_TOMBSTONES + 10 }, (_, index) => [
|
||||
`tab-${index}`,
|
||||
{ closedAt: NOW - index, worktreeId: WT }
|
||||
])
|
||||
)
|
||||
const capped = pruneClosedTerminalTabTombstones(overflowing, NOW)
|
||||
expect(Object.keys(capped)).toHaveLength(MAX_CLOSED_TERMINAL_TAB_TOMBSTONES)
|
||||
expect(capped['tab-0']).toBeDefined()
|
||||
expect(capped[`tab-${MAX_CLOSED_TERMINAL_TAB_TOMBSTONES + 9}`]).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('closed terminal tab tombstone acknowledgement', () => {
|
||||
const tombstones = { 'tab-1': { closedAt: NOW, worktreeId: WT } }
|
||||
|
||||
it('a snapshot carrying no revision acknowledges nothing', () => {
|
||||
expect(reconcile(tombstones, { hostRevision: undefined })).toEqual(tombstones)
|
||||
})
|
||||
|
||||
it('a snapshot with no row for the tombstone worktree acknowledges nothing', () => {
|
||||
const kept = reconcile(tombstones, { acknowledgedWorktreeIds: [], hostRevision: 9 })
|
||||
expect(kept['tab-1']).toEqual({ closedAt: NOW, worktreeId: WT })
|
||||
})
|
||||
|
||||
it('the first omitting snapshot only stamps the revision — it could predate the close', () => {
|
||||
const kept = reconcile(tombstones, { hostRevision: 4 })
|
||||
expect(kept['tab-1']).toEqual({ closedAt: NOW, worktreeId: WT, ackRevision: 4 })
|
||||
})
|
||||
|
||||
it('a strictly newer omitting snapshot retires the tombstone', () => {
|
||||
const stamped = reconcile(tombstones, { hostRevision: 4 })
|
||||
expect(reconcile(stamped, { hostRevision: 5 })['tab-1']).toBeUndefined()
|
||||
})
|
||||
|
||||
it('a repeat of the same revision does not retire it', () => {
|
||||
const stamped = reconcile(tombstones, { hostRevision: 4 })
|
||||
expect(reconcile(stamped, { hostRevision: 4 })['tab-1']).toBeDefined()
|
||||
})
|
||||
|
||||
it('a host that still lists the tab re-arms the watermark instead of retiring', () => {
|
||||
const stamped = reconcile(tombstones, { hostRevision: 4 })
|
||||
const stillListed = reconcile(stamped, { hostRevision: 7, hostKnownTabIds: ['tab-1'] })
|
||||
expect(stillListed['tab-1']).toEqual({ closedAt: NOW, worktreeId: WT, ackRevision: 7 })
|
||||
expect(reconcile(stillListed, { hostRevision: 7 })['tab-1']).toBeDefined()
|
||||
expect(reconcile(stillListed, { hostRevision: 8 })['tab-1']).toBeUndefined()
|
||||
})
|
||||
|
||||
it('a host revision that went backwards never retires', () => {
|
||||
const stamped = reconcile(tombstones, { hostRevision: 40 })
|
||||
const rewound = reconcile(stamped, { hostRevision: 1 })
|
||||
expect(rewound['tab-1']).toEqual({ closedAt: NOW, worktreeId: WT, ackRevision: 40 })
|
||||
})
|
||||
|
||||
it('a tab listed under a different worktree still counts as known to the host', () => {
|
||||
const stamped = reconcile(tombstones, { hostRevision: 4 })
|
||||
expect(
|
||||
reconcile(stamped, { hostRevision: 5, hostKnownTabIds: ['tab-1'] })['tab-1']
|
||||
).toBeDefined()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,111 @@
|
||||
import { z } from 'zod'
|
||||
|
||||
/** A client's own record that the user closed a terminal tab.
|
||||
*
|
||||
* Why it has to exist: the direct-SSH pull merge cannot tell "the host was never told about this
|
||||
* tab" from "the user closed it" by absence alone, so it keeps the tab (see the trade documented in
|
||||
* remote-workspace-session-merge.ts). When the `pty.kill` RPC rejects with a transport error the
|
||||
* close never reaches the host at all, the host keeps listing the tab, and every reconnect
|
||||
* re-inserts it. The tombstone is the missing close signal — first-party intent that outlives the
|
||||
* failed RPC — so the merge can drop that one tab without ever deleting a tab the host merely has
|
||||
* not heard about yet.
|
||||
*
|
||||
* Tab ids are uuids and reopen mints a fresh one, so a tombstoned id never legitimately returns. */
|
||||
export type ClosedTerminalTabTombstone = {
|
||||
closedAt: number
|
||||
worktreeId: string
|
||||
/** Newest host revision seen for this tab's scope since the close. Retirement needs a STRICTLY
|
||||
* newer snapshot that omits the tab, so a pull already in flight when the user closed cannot
|
||||
* acknowledge a close it predates. */
|
||||
ackRevision?: number
|
||||
}
|
||||
|
||||
export type ClosedTerminalTabTombstonesByTabId = Record<string, ClosedTerminalTabTombstone>
|
||||
|
||||
/** Colocated with the type so the two cannot drift. Must survive a relaunch: the whole point of a
|
||||
* tombstone is that it outlives a `pty.kill` the transport never delivered, and a restart is the
|
||||
* common case — it was omitted from the session schema once, and the map was silently stripped on
|
||||
* every launch. */
|
||||
export const closedTerminalTabTombstoneSchema = z.object({
|
||||
closedAt: z.number().int().nonnegative(),
|
||||
worktreeId: z.string().min(1),
|
||||
ackRevision: z.number().int().nonnegative().optional()
|
||||
})
|
||||
|
||||
/** Backstops only — host acknowledgement is the normal exit. These cover a target the user never
|
||||
* reconnects to, whose tombstones would otherwise never be retired. */
|
||||
export const CLOSED_TERMINAL_TAB_TOMBSTONE_TTL_MS = 30 * 24 * 60 * 60 * 1000
|
||||
export const MAX_CLOSED_TERMINAL_TAB_TOMBSTONES = 500
|
||||
|
||||
export function pruneClosedTerminalTabTombstones(
|
||||
map: ClosedTerminalTabTombstonesByTabId | undefined,
|
||||
now: number
|
||||
): ClosedTerminalTabTombstonesByTabId {
|
||||
const entries = Object.entries(map ?? {}).filter(
|
||||
([, tombstone]) => now - tombstone.closedAt <= CLOSED_TERMINAL_TAB_TOMBSTONE_TTL_MS
|
||||
)
|
||||
entries.sort(([, a], [, b]) => b.closedAt - a.closedAt)
|
||||
return Object.fromEntries(entries.slice(0, MAX_CLOSED_TERMINAL_TAB_TOMBSTONES))
|
||||
}
|
||||
|
||||
export function recordClosedTerminalTabTombstone(
|
||||
map: ClosedTerminalTabTombstonesByTabId | undefined,
|
||||
tabId: string,
|
||||
worktreeId: string,
|
||||
now: number
|
||||
): ClosedTerminalTabTombstonesByTabId {
|
||||
return pruneClosedTerminalTabTombstones({ ...map, [tabId]: { closedAt: now, worktreeId } }, now)
|
||||
}
|
||||
|
||||
function maxAckRevision(a: number | undefined, b: number | undefined): number | undefined {
|
||||
if (a === undefined) {
|
||||
return b
|
||||
}
|
||||
return b === undefined ? a : Math.max(a, b)
|
||||
}
|
||||
|
||||
export type ClosedTerminalTabTombstoneAck = {
|
||||
tombstones: ClosedTerminalTabTombstonesByTabId | undefined
|
||||
/** Worktrees this snapshot actually carries a tab row for. A worktree the snapshot says nothing
|
||||
* about — including one whose path never resolved to a local id — is not evidence of anything,
|
||||
* so its tombstones are left untouched. */
|
||||
acknowledgedWorktreeIds: ReadonlySet<string>
|
||||
/** Every tab id the snapshot lists, across all worktrees: an id the host still carries anywhere
|
||||
* has not been acknowledged, whichever worktree it now sits under. */
|
||||
hostKnownTabIds: ReadonlySet<string>
|
||||
hostRevision: number | undefined
|
||||
now: number
|
||||
}
|
||||
|
||||
/** Retires tombstones the host has demonstrably seen, and stamps the rest with the revision that
|
||||
* proved it had not yet.
|
||||
*
|
||||
* Retirement takes a snapshot that both covers the tombstone's worktree and is strictly newer than
|
||||
* the last one that did — one snapshot alone can be the pull that was already in flight when the
|
||||
* user closed. Absence never deletes here: a snapshot with no revision, or one carrying no row for
|
||||
* the worktree, retires nothing. */
|
||||
export function reconcileClosedTerminalTabTombstones({
|
||||
tombstones,
|
||||
acknowledgedWorktreeIds,
|
||||
hostKnownTabIds,
|
||||
hostRevision,
|
||||
now
|
||||
}: ClosedTerminalTabTombstoneAck): ClosedTerminalTabTombstonesByTabId {
|
||||
const pruned = pruneClosedTerminalTabTombstones(tombstones, now)
|
||||
if (hostRevision === undefined) {
|
||||
return pruned
|
||||
}
|
||||
const kept: ClosedTerminalTabTombstonesByTabId = {}
|
||||
for (const [tabId, tombstone] of Object.entries(pruned)) {
|
||||
if (!acknowledgedWorktreeIds.has(tombstone.worktreeId)) {
|
||||
kept[tabId] = tombstone
|
||||
continue
|
||||
}
|
||||
const observed = tombstone.ackRevision
|
||||
if (!hostKnownTabIds.has(tabId) && observed !== undefined && hostRevision > observed) {
|
||||
continue
|
||||
}
|
||||
kept[tabId] = { ...tombstone, ackRevision: maxAckRevision(observed, hostRevision) }
|
||||
}
|
||||
return kept
|
||||
}
|
||||
@@ -0,0 +1,14 @@
|
||||
import { z } from 'zod'
|
||||
|
||||
import { terminalTabIdSchema } from './terminal-tab-id-schema'
|
||||
|
||||
/** The value schema for `WorkspaceSessionState.terminalSurfaceTombstonesByPaneKey`. Extracted from
|
||||
* workspace-session-schema.ts, which is at its max-lines limit. */
|
||||
export const terminalSurfaceTombstoneSchema = z.object({
|
||||
worktreeId: z.string(),
|
||||
parentTabId: terminalTabIdSchema,
|
||||
leafId: z.string(),
|
||||
ptyId: z.string(),
|
||||
incarnationId: z.string().min(1).max(128),
|
||||
retiredAt: z.number().finite().nonnegative()
|
||||
})
|
||||
@@ -0,0 +1,10 @@
|
||||
import { z } from 'zod'
|
||||
|
||||
import { isValidTerminalTabId } from './terminal-tab-id'
|
||||
|
||||
/** Shared so the session schema and the tombstone value schemas validate tab ids identically.
|
||||
* Extracted from workspace-session-schema.ts, which is at its max-lines limit. */
|
||||
export const terminalTabIdSchema = z
|
||||
.string()
|
||||
.min(1)
|
||||
.refine(isValidTerminalTabId, 'terminal tab id must not contain ":"')
|
||||
@@ -0,0 +1,116 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { z } from 'zod'
|
||||
|
||||
import { parseWorkspaceSession, workspaceSessionStateSchema } from './workspace-session-schema'
|
||||
import type { WorkspaceSessionState } from './workspace-session-state-types'
|
||||
|
||||
/**
|
||||
* `workspaceSessionStateSchema` is the load boundary for BOTH partitions —
|
||||
* `normalize-loaded-state-collections.ts` for `local`, `workspace-session-partitions.ts` for
|
||||
* `ssh:<target>`. Zod strips unknown keys and the write side does not validate, so a field declared
|
||||
* on WorkspaceSessionState but missing here reaches disk and is silently discarded on the next
|
||||
* launch.
|
||||
*
|
||||
* That failure is invisible to any test living inside one app process, which is how the closed-tab
|
||||
* tombstone map shipped unpersisted: its unit suites, its e2e oracle and an A/B control were all
|
||||
* green, and the fix still did not survive a quit-and-relaunch.
|
||||
*
|
||||
* Two sibling tables (`profile-project-session-field-disposition.ts`,
|
||||
* `workspace-session-host-field-ownership.ts`) already pin themselves with
|
||||
* `satisfies Record<keyof WorkspaceSessionState, ...>`. This schema had no such guard and is the one
|
||||
* that fell behind, so the ratchet below is the same shape.
|
||||
*/
|
||||
const PERSISTED_WORKSPACE_SESSION_FIELDS = {
|
||||
activeRepoId: true,
|
||||
activeWorkspaceKey: true,
|
||||
activeWorkspaceExecutionHostId: true,
|
||||
activeWorktreeId: true,
|
||||
activeTabId: true,
|
||||
tabsByWorktree: true,
|
||||
terminalLayoutsByTabId: true,
|
||||
activeWorktreeIdsOnShutdown: true,
|
||||
openFilesByWorktree: true,
|
||||
activeFileIdByWorktree: true,
|
||||
markdownFrontmatterVisible: true,
|
||||
browserTabsByWorktree: true,
|
||||
browserPagesByWorkspace: true,
|
||||
activeBrowserTabIdByWorktree: true,
|
||||
clientHostedBrowserPagesByWorktree: true,
|
||||
clientHostedBrowserCloseIntentsByEnvironment: true,
|
||||
activeTabTypeByWorktree: true,
|
||||
browserUrlHistory: true,
|
||||
activeTabIdByWorktree: true,
|
||||
unifiedTabs: true,
|
||||
tabGroups: true,
|
||||
tabGroupLayouts: true,
|
||||
activeGroupIdByWorktree: true,
|
||||
activeConnectionIdsAtShutdown: true,
|
||||
remoteSessionIdsByTabId: true,
|
||||
lastVisitedAtByWorktreeId: true,
|
||||
defaultTerminalTabsAppliedByWorktreeId: true,
|
||||
sleepingAgentSessionsByPaneKey: true,
|
||||
terminalPtyIncarnationsByPaneKey: true,
|
||||
terminalTopologyRevisionByRepoId: true,
|
||||
terminalSurfaceTombstonesByPaneKey: true,
|
||||
closedTerminalTabTombstonesByTabId: true
|
||||
} satisfies Record<keyof WorkspaceSessionState, true>
|
||||
|
||||
const MINIMAL_SESSION = {
|
||||
activeRepoId: null,
|
||||
activeWorktreeId: null,
|
||||
activeTabId: null,
|
||||
tabsByWorktree: {},
|
||||
terminalLayoutsByTabId: {}
|
||||
}
|
||||
|
||||
describe('workspaceSessionStateSchema field coverage', () => {
|
||||
it('parses every field declared on WorkspaceSessionState', () => {
|
||||
// The runtime half. `satisfies` above already makes a forgotten field a compile error; this
|
||||
// reports it by name, and catches the reverse case where the list is updated but the schema
|
||||
// is not.
|
||||
// Why the cast: the export is annotated `z.ZodType<WorkspaceSessionState>`, which hides
|
||||
// `.shape`. The value is a z.object; this reads its keys without widening the public type.
|
||||
const schemaKeys = new Set(
|
||||
Object.keys(
|
||||
(workspaceSessionStateSchema as unknown as z.ZodObject<Record<string, z.ZodTypeAny>>).shape
|
||||
)
|
||||
)
|
||||
const missing = Object.keys(PERSISTED_WORKSPACE_SESSION_FIELDS).filter(
|
||||
(field) => !schemaKeys.has(field)
|
||||
)
|
||||
|
||||
expect(missing, `declared on WorkspaceSessionState but absent from the load schema`).toEqual([])
|
||||
})
|
||||
|
||||
it('round-trips the closed-tab tombstone map through parseWorkspaceSession', () => {
|
||||
// The regression this file exists for. Recorded on close, written to disk, stripped on the next
|
||||
// launch — so a close the transport never delivered resurrected after a quit-and-relaunch,
|
||||
// which is precisely the reported shape ("every day I open orca and it opens more tabs").
|
||||
const parsed = parseWorkspaceSession({
|
||||
...MINIMAL_SESSION,
|
||||
closedTerminalTabTombstonesByTabId: {
|
||||
'tab-1': { closedAt: 1_700_000_000_000, worktreeId: 'repo:wt-1', ackRevision: 4 }
|
||||
}
|
||||
})
|
||||
|
||||
expect(parsed.ok).toBe(true)
|
||||
expect(parsed.ok && parsed.value.closedTerminalTabTombstonesByTabId).toEqual({
|
||||
'tab-1': { closedAt: 1_700_000_000_000, worktreeId: 'repo:wt-1', ackRevision: 4 }
|
||||
})
|
||||
})
|
||||
|
||||
it('salvages a malformed tombstone entry rather than dropping the whole map', () => {
|
||||
const parsed = parseWorkspaceSession({
|
||||
...MINIMAL_SESSION,
|
||||
closedTerminalTabTombstonesByTabId: {
|
||||
'tab-good': { closedAt: 1, worktreeId: 'repo:wt-1' },
|
||||
'tab-bad': { closedAt: 'nope', worktreeId: 'repo:wt-1' }
|
||||
}
|
||||
})
|
||||
|
||||
expect(parsed.ok).toBe(true)
|
||||
expect(
|
||||
Object.keys((parsed.ok && parsed.value.closedTerminalTabTombstonesByTabId) || {})
|
||||
).toEqual(['tab-good'])
|
||||
})
|
||||
})
|
||||
@@ -12,12 +12,14 @@
|
||||
* Only a payload that is not a session at all falls back to defaults.
|
||||
*/
|
||||
import { z } from 'zod'
|
||||
import { closedTerminalTabTombstoneSchema } from './closed-terminal-tab-tombstones'
|
||||
import type { WorkspaceKey } from './folder-workspace-types'
|
||||
import type { TabGroupLayoutNode } from './tab-types'
|
||||
import type { TerminalPaneLayoutNode } from './terminal-tab-types'
|
||||
import type { TuiAgent } from './tui-agent'
|
||||
import type { WorkspaceSessionState } from './workspace-session-state-types'
|
||||
import { isValidTerminalTabId } from './terminal-tab-id'
|
||||
import { terminalTabIdSchema } from './terminal-tab-id-schema'
|
||||
import { terminalSurfaceTombstoneSchema } from './terminal-surface-tombstone-schema'
|
||||
import { parseExecutionHostId, type ExecutionHostId } from './execution-host'
|
||||
import { isTuiAgent } from './tui-agent-config'
|
||||
import { isWorkspaceKey } from './workspace-scope'
|
||||
@@ -35,10 +37,6 @@ import { salvagedField, salvagedOptional, salvagingArray, salvagingRecord } from
|
||||
// ─── Terminal pane layout (recursive) ───────────────────────────────
|
||||
|
||||
const terminalPaneSplitDirectionSchema = z.enum(['vertical', 'horizontal'])
|
||||
const terminalTabIdSchema = z
|
||||
.string()
|
||||
.min(1)
|
||||
.refine(isValidTerminalTabId, 'terminal tab id must not contain ":"')
|
||||
const workspaceKeySchema = z.custom<WorkspaceKey>(
|
||||
(value) => typeof value === 'string' && isWorkspaceKey(value)
|
||||
)
|
||||
@@ -190,15 +188,6 @@ const tabGroupLayoutNodeSchema: z.ZodType<TabGroupLayoutNode> = z.lazy(() =>
|
||||
|
||||
// ─── Workspace session ──────────────────────────────────────────────
|
||||
|
||||
const terminalSurfaceTombstoneSchema = z.object({
|
||||
worktreeId: z.string(),
|
||||
parentTabId: terminalTabIdSchema,
|
||||
leafId: z.string(),
|
||||
ptyId: z.string(),
|
||||
incarnationId: z.string().min(1).max(128),
|
||||
retiredAt: z.number().finite().nonnegative()
|
||||
})
|
||||
|
||||
const worktreeIdSchema = z.string()
|
||||
|
||||
export const workspaceSessionStateSchema: z.ZodType<WorkspaceSessionState> = z.object({
|
||||
@@ -314,6 +303,10 @@ export const workspaceSessionStateSchema: z.ZodType<WorkspaceSessionState> = z.o
|
||||
terminalSurfaceTombstonesByPaneKey: salvagedOptional(
|
||||
'terminalSurfaceTombstonesByPaneKey',
|
||||
salvagingRecord(z.string(), terminalSurfaceTombstoneSchema)
|
||||
),
|
||||
closedTerminalTabTombstonesByTabId: salvagedOptional(
|
||||
'closedTerminalTabTombstonesByTabId',
|
||||
salvagingRecord(terminalTabIdSchema, closedTerminalTabTombstoneSchema)
|
||||
)
|
||||
})
|
||||
|
||||
|
||||
@@ -6,6 +6,7 @@ import type { TerminalLayoutSnapshot, TerminalTab } from './terminal-tab-types'
|
||||
import type { BrowserHistoryEntry, BrowserPage, BrowserWorkspace } from './browser-workspace-types'
|
||||
import type { ClientHostedBrowserCloseIntent } from './client-hosted-browser-close-intent'
|
||||
import type { PersistedClientHostedBrowserPage } from './client-hosted-browser-page-record'
|
||||
import type { ClosedTerminalTabTombstonesByTabId } from './closed-terminal-tab-tombstones'
|
||||
|
||||
/** Minimal subset of OpenFile persisted across restarts.
|
||||
* Only edit-mode files are saved — diffs, conflict reviews, and other
|
||||
@@ -124,6 +125,9 @@ export type WorkspaceSessionState = {
|
||||
retiredAt: number
|
||||
}
|
||||
>
|
||||
/** Terminal tabs this client watched the user close, kept until the host's own snapshot stops
|
||||
* listing them. See shared/closed-terminal-tab-tombstones.ts for why absence alone cannot say it. */
|
||||
closedTerminalTabTombstonesByTabId?: ClosedTerminalTabTombstonesByTabId
|
||||
}
|
||||
|
||||
export type WorkspaceSessionPatch = Partial<WorkspaceSessionState>
|
||||
|
||||
@@ -0,0 +1,198 @@
|
||||
import type { Page, TestInfo } from '@stablyai/playwright-test'
|
||||
import { test, expect } from './helpers/orca-app'
|
||||
import { waitForActiveWorktree, waitForSessionReady } from './helpers/store'
|
||||
import { waitForActivePanePtyId, waitForActiveTerminalManager } from './helpers/terminal'
|
||||
import { createRemoteTerminalTab } from './helpers/docker-ssh-relay-terminal-tabs'
|
||||
import {
|
||||
cleanupDockerSshRelayTarget,
|
||||
startDockerSshRelayTarget,
|
||||
type DockerSshRelayTarget
|
||||
} from './helpers/docker-ssh-relay-target'
|
||||
import {
|
||||
readDockerSshRelayProcessSnapshot,
|
||||
terminateDockerSshRelay
|
||||
} from './helpers/docker-ssh-relay-processes'
|
||||
import {
|
||||
connectDockerSshRelayTarget,
|
||||
disconnectDockerSshRelayTarget,
|
||||
reconnectDisconnectedDockerSshRelayTarget
|
||||
} from './helpers/docker-ssh-relay-connection'
|
||||
|
||||
const RUN_DOCKER_SSH = process.env.ORCA_E2E_SSH_DOCKER === '1'
|
||||
const DROP_CYCLES = 3
|
||||
|
||||
test.use({ seedTestRepo: false })
|
||||
|
||||
async function readWorktreeTabIds(page: Page, worktreeId: string): Promise<string[]> {
|
||||
return page.evaluate(
|
||||
(id) => (window.__store?.getState().tabsByWorktree[id] ?? []).map((tab) => tab.id),
|
||||
worktreeId
|
||||
)
|
||||
}
|
||||
|
||||
/** Poll until the tab set stops changing, so a resurrection that lands late still counts. */
|
||||
async function waitForSettledTabIds(page: Page, worktreeId: string): Promise<string[]> {
|
||||
let latest: string[] = []
|
||||
let previousKey = ''
|
||||
let agreements = 0
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
latest = await readWorktreeTabIds(page, worktreeId)
|
||||
const key = latest.join()
|
||||
agreements = key === previousKey ? agreements + 1 : 0
|
||||
previousKey = key
|
||||
return agreements
|
||||
},
|
||||
{ timeout: 60_000, intervals: [1_000], message: 'the tab set never stopped changing' }
|
||||
)
|
||||
.toBeGreaterThanOrEqual(3)
|
||||
return latest
|
||||
}
|
||||
|
||||
/**
|
||||
* Kill the relay daemon so the next RPC rejects with a transport-class error.
|
||||
*
|
||||
* Why this and not a disconnect: `pty.kill` has to FAIL, not succeed against a dead session.
|
||||
* A transport rejection ("Multiplexer disposed" / CONNECTION_LOST) does not match
|
||||
* isPtyAlreadyGoneError, so the lease is never marked terminated and nothing retries it —
|
||||
* that unterminated lease is what the next reattach mistakes for a live PTY.
|
||||
*/
|
||||
function dropRelayTransport(target: DockerSshRelayTarget): void {
|
||||
const snapshot = readDockerSshRelayProcessSnapshot(target)
|
||||
if (!snapshot) {
|
||||
throw new Error('No Docker SSH relay process group to terminate')
|
||||
}
|
||||
terminateDockerSshRelay(target, snapshot)
|
||||
}
|
||||
|
||||
async function dumpAuthority(page: Page, worktreeId: string, label: string): Promise<void> {
|
||||
const d = await page.evaluate(async (id) => {
|
||||
const persisted = await window.api.session.get()
|
||||
const state = window.__store?.getState()
|
||||
const repoId = Object.entries(state?.worktreesByRepo ?? {}).find(([, ws]) =>
|
||||
ws.some((w) => w.id === id)
|
||||
)?.[0]
|
||||
return {
|
||||
repoId: repoId?.slice(0, 8) ?? null,
|
||||
topologyRev: persisted.terminalTopologyRevisionByRepoId ?? null,
|
||||
tombstones: Object.keys(persisted.terminalSurfaceTombstonesByPaneKey ?? {}).length,
|
||||
storeTabs: (state?.tabsByWorktree[id] ?? []).length
|
||||
}
|
||||
}, worktreeId)
|
||||
console.log(`[auth ${label}] ${JSON.stringify(d)}`)
|
||||
}
|
||||
|
||||
async function closeTerminalTab(page: Page, tabId: string): Promise<void> {
|
||||
await page.evaluate((id) => {
|
||||
const state = window.__store?.getState()
|
||||
if (!state) {
|
||||
throw new Error('Store unavailable')
|
||||
}
|
||||
state.closeTab(id)
|
||||
}, tabId)
|
||||
}
|
||||
|
||||
/**
|
||||
* Run N cycles of: make a tab, disrupt the transport, close the tab, reconnect.
|
||||
*
|
||||
* `disrupt` is the only variable — the two callers differ solely in HOW the transport goes away,
|
||||
* so a difference in outcome is attributable to that and nothing else.
|
||||
*/
|
||||
async function runResurrectionCycles(
|
||||
page: Page,
|
||||
testInfo: TestInfo,
|
||||
disrupt: (target: DockerSshRelayTarget, targetId: string) => Promise<void> | void
|
||||
): Promise<void> {
|
||||
let target: DockerSshRelayTarget | null = null
|
||||
try {
|
||||
target = startDockerSshRelayTarget(testInfo)
|
||||
await waitForSessionReady(page)
|
||||
const remote = await connectDockerSshRelayTarget(page, target)
|
||||
await expect
|
||||
.poll(() => waitForActiveWorktree(page), { timeout: 30_000 })
|
||||
.toBe(remote.worktreeId)
|
||||
await waitForActiveTerminalManager(page, 60_000)
|
||||
await waitForActivePanePtyId(page, 60_000)
|
||||
const baseline = await waitForSettledTabIds(page, remote.worktreeId)
|
||||
|
||||
const perCycle: { closedTabId: string; afterIds: string[] }[] = []
|
||||
for (let cycle = 0; cycle < DROP_CYCLES; cycle += 1) {
|
||||
// Created while the transport is healthy — the disruption has to land between close and
|
||||
// reattach, not before the tab has a PTY to leave a lease behind.
|
||||
const beforeCreate = await waitForSettledTabIds(page, remote.worktreeId)
|
||||
await createRemoteTerminalTab(page, remote.worktreeId)
|
||||
const withExtra = await waitForSettledTabIds(page, remote.worktreeId)
|
||||
// Diffed against the PREVIOUS cycle's tabs, not the baseline: once a cycle resurrects a tab,
|
||||
// a baseline diff picks that survivor instead of the tab this cycle just made, and every
|
||||
// later cycle would close the same stale tab and measure nothing.
|
||||
const closedTabId = withExtra.find((tabId) => !beforeCreate.includes(tabId))
|
||||
if (!closedTabId) {
|
||||
throw new Error('The extra SSH tab was never added')
|
||||
}
|
||||
|
||||
await dumpAuthority(page, remote.worktreeId, `cycle${cycle + 1}-before-close`)
|
||||
await disrupt(target, remote.targetId)
|
||||
await closeTerminalTab(page, closedTabId)
|
||||
await reconnectDisconnectedDockerSshRelayTarget(page, remote.targetId)
|
||||
await waitForActiveTerminalManager(page, 60_000)
|
||||
perCycle.push({
|
||||
closedTabId,
|
||||
afterIds: await waitForSettledTabIds(page, remote.worktreeId)
|
||||
})
|
||||
await dumpAuthority(page, remote.worktreeId, `cycle${cycle + 1}-after-reconnect`)
|
||||
}
|
||||
|
||||
const growth = perCycle
|
||||
.map(
|
||||
(entry, index) =>
|
||||
`drop${index + 1}=${entry.afterIds.length}${entry.afterIds.includes(entry.closedTabId) ? ' (closed tab returned)' : ''}`
|
||||
)
|
||||
.join(' ')
|
||||
const summary = `baseline=${baseline.length} ${growth}`
|
||||
for (const [index, entry] of perCycle.entries()) {
|
||||
expect(
|
||||
entry.afterIds,
|
||||
`drop ${index + 1} resurrected the closed tab ${entry.closedTabId}: ${summary}`
|
||||
).not.toContain(entry.closedTabId)
|
||||
}
|
||||
expect(
|
||||
perCycle.map((entry) => entry.afterIds.length),
|
||||
`tabs accumulated across dropped-transport closes: ${summary}`
|
||||
).toEqual(perCycle.map(() => baseline.length))
|
||||
} finally {
|
||||
cleanupDockerSshRelayTarget(target)
|
||||
}
|
||||
}
|
||||
|
||||
test.describe('SSH lost kill tab resurrection', () => {
|
||||
test.skip(!RUN_DOCKER_SSH, 'Set ORCA_E2E_SSH_DOCKER=1 to run Docker-backed SSH tests.')
|
||||
test.skip(process.platform === 'win32', 'Docker SSH restore uses POSIX SSH tooling.')
|
||||
|
||||
// STA-3374. A tab closed while the transport is down leaves an unterminated remote lease: the
|
||||
// rejected `pty.kill` is a transport error, not an already-gone one, so nothing retires it. The
|
||||
// relay also restarts its pty counter at pty-1, so a later tab is handed the same id and
|
||||
// upsertSshRemotePtyLease — keyed on (targetId, ptyId) alone — collides with that stale lease
|
||||
// instead of minting a fresh one. The next reattach then re-mints the tab through
|
||||
// pty-binding-persistence.ts:145-160, and the resurrected tab never retires.
|
||||
test('does not resurrect tabs whose kill was lost to a killed relay daemon', async ({
|
||||
orcaPage
|
||||
}, testInfo) => {
|
||||
test.setTimeout(600_000)
|
||||
await runResurrectionCycles(orcaPage, testInfo, (target) => {
|
||||
dropRelayTransport(target)
|
||||
})
|
||||
})
|
||||
|
||||
// The same close, reached the way a user reaches it: disconnect the host, close the tab, come
|
||||
// back. No daemon is killed. If this resurrects too, the bug needs no process death at all — a
|
||||
// laptop lid and a dropped link are enough.
|
||||
test('does not resurrect tabs closed while the host is disconnected', async ({
|
||||
orcaPage
|
||||
}, testInfo) => {
|
||||
test.setTimeout(600_000)
|
||||
await runResurrectionCycles(orcaPage, testInfo, async (_target, targetId) => {
|
||||
await disconnectDockerSshRelayTarget(orcaPage, targetId)
|
||||
})
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user