mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
fix(terminal): carry a parked pane's scrollback through the mirrored-layout rebuild
Found in review of this PR by rc-ssh-remoting. chooseRemoteTerminalLayout rebuilds a mirrored tab's layout from the host's picture and never carried buffersByLeafId or scrollbackRefsByLeafId forward, though it already receives existingLayout. The host publishes no scrollback of its own, so ANY session-inventory frame landing between park and reveal dropped the only client-side copy: the rebuild is bufferless, terminalLayoutEqual compares buffers so the write is not bailed out, and apply-terminal-records assigns it wholesale. Measured before the fix: 336 bytes captured at park, 0 after one forced frame, blank pane on reveal. After: 411 bytes survive the frame and the reveal repaints. The e2e passed either way because no frame happened to land in its window, so it was not covering the destroying event. It now forces one inside the park -> reveal window and asserts the capture survives it. An identical fix was written and reverted earlier in this branch as 'no measurable effect' — that measurement ran on a harness deleting the client profile between launches, so nothing downstream of persistence could register. It was never actually tested.
This commit is contained in:
@@ -6,6 +6,7 @@ import type { TerminalLayoutSnapshot, TerminalTab } from '../../../../shared/ter
|
||||
import { defaultAgentChatLabel } from '../../../../shared/agent-session-chat-label'
|
||||
import { sanitizeTerminalLayoutPaneTitlesForLabels } from '@/lib/terminal-pane-title-sanitization'
|
||||
import { resolveTerminalLayoutRoot } from '../remote-terminal-layout-resolution'
|
||||
import { retainLocalScrollbackInRemoteLayout } from '@/components/terminal-pane/remote-layout-scrollback-retention'
|
||||
import { getRemoteRuntimePtyEnvironmentId } from '../runtime-terminal-stream'
|
||||
import {
|
||||
HOST_TERMINAL_SURFACE_SEPARATOR,
|
||||
@@ -200,7 +201,13 @@ export function chooseRemoteTerminalLayout(
|
||||
: parentLayout?.expandedLeafId && knownLeafIds.has(parentLayout.expandedLeafId)
|
||||
? parentLayout.expandedLeafId
|
||||
: null
|
||||
return {
|
||||
// Why retained: this rebuilds the layout from the host's picture, and the host publishes no
|
||||
// scrollback of its own — a parked remote pane's bytes live only in the client's copy. Without
|
||||
// this, ANY inventory frame landing between park and reveal drops the only copy: the rebuild is
|
||||
// bufferless, terminalLayoutEqual compares buffers so the write is not bailed out, and
|
||||
// apply-terminal-records assigns it wholesale. Structure still comes from the host; only bytes
|
||||
// for leaves the host itself names are carried over.
|
||||
return retainLocalScrollbackInRemoteLayout(existingLayout, {
|
||||
// Why: host parentLayout is authoritative for split direction; else keep the prior client tree, then degenerate — never re-guess a direction.
|
||||
root: resolveTerminalLayoutRoot({
|
||||
authoritativeRoot: parentLayout?.root,
|
||||
@@ -216,7 +223,7 @@ export function chooseRemoteTerminalLayout(
|
||||
ptyIdsByLeafId,
|
||||
// Why: surface.title is the tab/PTY label, not a pane title; restoring it as one renders a fake title bar. Only host layout titles are real pane titles.
|
||||
...(parentLayout?.titlesByLeafId ? { titlesByLeafId: parentLayout.titlesByLeafId } : {})
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
export function shouldReplaceTerminalTab(
|
||||
|
||||
@@ -174,6 +174,45 @@ async function runParkRevealScenario(
|
||||
)
|
||||
console.log(`[parked-scrollback] park-diagnostics ${JSON.stringify(parkDiagnostics)}`)
|
||||
|
||||
// Why a forced inventory frame: without one this spec passes whether or not the mirrored-layout
|
||||
// rebuild carries the capture, because no frame happens to land in its window. A host frame
|
||||
// rebuilds the tab's layout bufferless; terminalLayoutEqual compares buffers, so the write is
|
||||
// not bailed out and apply-terminal-records assigns it wholesale. That wipes the only
|
||||
// client-side copy. This is the destroying event, so it belongs inside the window under test.
|
||||
const probeTab = await createPairedHostTerminal(
|
||||
clientPage,
|
||||
environmentId,
|
||||
worktreeId,
|
||||
fixtureCommand()
|
||||
)
|
||||
createdTerminals.push(probeTab.terminal)
|
||||
await expect
|
||||
.poll(
|
||||
() =>
|
||||
clientPage.evaluate(
|
||||
(id) => (window.__store?.getState().tabsByWorktree[id] ?? []).map((tab) => tab.id),
|
||||
worktreeId
|
||||
),
|
||||
{
|
||||
timeout: 60_000,
|
||||
message: 'client never mirrored the probe tab (no inventory frame landed)'
|
||||
}
|
||||
)
|
||||
.toContain(probeTab.webTabId)
|
||||
const afterInventoryFrame = await clientPage.evaluate((webTabId) => {
|
||||
const layout = window.__store?.getState().terminalLayoutsByTabId?.[webTabId]
|
||||
return {
|
||||
leafIds: Object.keys(layout?.buffersByLeafId ?? {}),
|
||||
length: Object.values(layout?.buffersByLeafId ?? {}).join('').length,
|
||||
layoutKnown: layout !== undefined
|
||||
}
|
||||
}, target.webTabId)
|
||||
console.log(`[parked-scrollback] after-inventory-frame ${JSON.stringify(afterInventoryFrame)}`)
|
||||
expect(
|
||||
{ survivedInventoryFrame: afterInventoryFrame.length > 0 },
|
||||
'a host inventory frame wiped the park capture before the reveal'
|
||||
).toEqual({ survivedInventoryFrame: true })
|
||||
|
||||
await openPairedClientTab(clientPage, worktreeId, target.webTabId)
|
||||
const tokenAfterReveal = await waitForPairedPaneMarker(
|
||||
clientPage,
|
||||
|
||||
Reference in New Issue
Block a user