mirror of
https://github.com/stablyai/orca.git
synced 2026-09-29 16:02:50 +00:00
fix(runtime): stop re-seeding a runtime-owned workspace the user emptied
Focusing a workspace owned by a remote runtime created a terminal every time, and sometimes two. The mirror could never record the closed-last-terminal state. A host snapshot with no terminals produced `nextTerminalTabs === null`, which `withWorktreeEntry` turns into a deleted key -- and a missing row is exactly how every seeder spells "never initialized" (initial-terminal.ts). Keep an explicit empty row instead, so the remote path reads the same tombstone the local one already honours. A worktree that never had a terminal still gets no row, because `sameTerminalTabs` treats a missing row and an empty one as equal; removal frames and synthesized unpublished frames keep deleting, since neither is evidence the user emptied anything. The duplicate had a second cause. `requestedInitialTerminal` was a `let` inside the session-tabs subscription closure, so "one focus creates at most one terminal" held only for as long as that closure lived. Its effect re-runs whenever the environment, connection generation, pairing revision, or session-ready flag settles -- all of which move during a workspace switch -- so a second closure re-armed the flag while the first create was still in flight. That is the asymmetry in the report: one terminal when arriving from the landing screen, two when arriving from another workspace. Latch the bootstrap per worktree in a module-scoped set instead, modelled on web-runtime-wake-terminal-respawn.ts, released when the create settles. The closure flag stays alongside it so a failed create still does not retry on every later frame of the same subscription. Fixes STA-6173.
This commit is contained in:
@@ -0,0 +1,38 @@
|
||||
/**
|
||||
* Which runtime-owned workspaces already have an initial-terminal bootstrap in flight.
|
||||
*
|
||||
* The bootstrap used to be latched by a `let` inside the session-tabs subscription closure, which
|
||||
* made "one focus creates at most one terminal" true only for as long as that one closure lived.
|
||||
* Its effect re-runs whenever the environment, connection generation, pairing revision, or
|
||||
* session-ready flag settles — all of which move during a workspace switch — so a second closure
|
||||
* re-armed the flag while the first create was still in flight and seeded a second terminal
|
||||
* (STA-6173). This latch outlives the closures. It is released when the create settles: by then the
|
||||
* create has awaited its own snapshot refresh, so the host's tab is mirrored and the predicate
|
||||
* declines on its own, while a failed create is free to be retried by a later focus.
|
||||
*/
|
||||
const initialTerminalBootstrapInFlightByWorktree = new Set<string>()
|
||||
|
||||
export function isWebRuntimeInitialTerminalBootstrapInFlight(worktreeId: string): boolean {
|
||||
return initialTerminalBootstrapInFlightByWorktree.has(worktreeId)
|
||||
}
|
||||
|
||||
/** Claims the bootstrap for this worktree; false when another closure already holds it. */
|
||||
export function beginWebRuntimeInitialTerminalBootstrap(worktreeId: string): boolean {
|
||||
if (initialTerminalBootstrapInFlightByWorktree.has(worktreeId)) {
|
||||
return false
|
||||
}
|
||||
initialTerminalBootstrapInFlightByWorktree.add(worktreeId)
|
||||
return true
|
||||
}
|
||||
|
||||
export function endWebRuntimeInitialTerminalBootstrap(worktreeId: string): void {
|
||||
initialTerminalBootstrapInFlightByWorktree.delete(worktreeId)
|
||||
}
|
||||
|
||||
export function clearAllWebRuntimeInitialTerminalBootstraps(): void {
|
||||
initialTerminalBootstrapInFlightByWorktree.clear()
|
||||
}
|
||||
|
||||
export function resetWebRuntimeInitialTerminalBootstrapForTests(): void {
|
||||
clearAllWebRuntimeInitialTerminalBootstraps()
|
||||
}
|
||||
@@ -0,0 +1,138 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
UNPUBLISHED_WORKTREE_PUBLICATION_EPOCH,
|
||||
type RuntimeMobileSessionTabsRemovedResult
|
||||
} from '../../../shared/runtime-types'
|
||||
import type { TerminalTab } from '../../../shared/terminal-tab-types'
|
||||
import { applyWebSessionTabsSnapshot, type WebSessionTabsSyncState } from './web-session-tabs-sync'
|
||||
import {
|
||||
ENV,
|
||||
HOST_SURFACE_ID,
|
||||
LEAF_ID,
|
||||
NOW,
|
||||
WT,
|
||||
makeSnapshot,
|
||||
makeState,
|
||||
resetWebSessionTabsSyncTestState
|
||||
} from './web-session-tabs-sync-test-harness'
|
||||
|
||||
vi.mock('../store', () => ({
|
||||
useAppStore: {
|
||||
setState: vi.fn()
|
||||
}
|
||||
}))
|
||||
|
||||
const hostTerminalSnapshot = (): ReturnType<typeof makeSnapshot> =>
|
||||
makeSnapshot([
|
||||
{
|
||||
type: 'terminal',
|
||||
id: HOST_SURFACE_ID,
|
||||
parentTabId: 'host-tab-1',
|
||||
leafId: LEAF_ID,
|
||||
title: 'Terminal',
|
||||
status: 'ready',
|
||||
terminal: 'terminal-1',
|
||||
isActive: true
|
||||
}
|
||||
])
|
||||
|
||||
const emptySnapshot = (
|
||||
overrides: Partial<ReturnType<typeof makeSnapshot>> = {}
|
||||
): ReturnType<typeof makeSnapshot> =>
|
||||
makeSnapshot([], {
|
||||
snapshotVersion: 2,
|
||||
activeGroupId: null,
|
||||
activeTabId: null,
|
||||
activeTabType: null,
|
||||
...overrides
|
||||
})
|
||||
|
||||
/**
|
||||
* STA-6173. The seeders read a missing terminal row as "never initialized" and an explicit empty
|
||||
* one as "the user closed the last terminal" (initial-terminal.ts). The mirror used to delete the
|
||||
* key, so a runtime-owned workspace could never record the second state and was re-seeded on every
|
||||
* focus.
|
||||
*/
|
||||
describe('applyWebSessionTabsSnapshot emptied-workspace tombstone', () => {
|
||||
beforeEach(resetWebSessionTabsSyncTestState)
|
||||
|
||||
it('leaves an explicit empty row when the host retracts the last terminal', () => {
|
||||
const mirrored = applyWebSessionTabsSnapshot(
|
||||
makeState(),
|
||||
hostTerminalSnapshot(),
|
||||
ENV,
|
||||
NOW
|
||||
) as Partial<WebSessionTabsSyncState>
|
||||
const mirroredTabs = mirrored.tabsByWorktree?.[WT] as TerminalTab[]
|
||||
expect(mirroredTabs).toHaveLength(1)
|
||||
|
||||
const patch = applyWebSessionTabsSnapshot(
|
||||
makeState({ tabsByWorktree: { [WT]: mirroredTabs } }),
|
||||
emptySnapshot(),
|
||||
ENV,
|
||||
NOW
|
||||
) as Partial<WebSessionTabsSyncState>
|
||||
|
||||
expect(patch.tabsByWorktree).toBeDefined()
|
||||
expect(Object.hasOwn(patch.tabsByWorktree!, WT)).toBe(true)
|
||||
expect(patch.tabsByWorktree![WT]).toEqual([])
|
||||
})
|
||||
|
||||
it('does not invent a row for a workspace the host has never had terminals in', () => {
|
||||
const patch = applyWebSessionTabsSnapshot(
|
||||
makeState(),
|
||||
emptySnapshot(),
|
||||
ENV,
|
||||
NOW
|
||||
) as Partial<WebSessionTabsSyncState>
|
||||
|
||||
expect(patch.tabsByWorktree === undefined || !Object.hasOwn(patch.tabsByWorktree, WT)).toBe(
|
||||
true
|
||||
)
|
||||
})
|
||||
|
||||
it('drops the row on a worktree removal frame instead of tombstoning a workspace that is gone', () => {
|
||||
const mirrored = applyWebSessionTabsSnapshot(
|
||||
makeState(),
|
||||
hostTerminalSnapshot(),
|
||||
ENV,
|
||||
NOW
|
||||
) as Partial<WebSessionTabsSyncState>
|
||||
const mirroredTabs = mirrored.tabsByWorktree?.[WT] as TerminalTab[]
|
||||
|
||||
const patch = applyWebSessionTabsSnapshot(
|
||||
makeState({ tabsByWorktree: { [WT]: mirroredTabs } }),
|
||||
{ ...emptySnapshot(), removed: true } as RuntimeMobileSessionTabsRemovedResult,
|
||||
ENV,
|
||||
NOW
|
||||
) as Partial<WebSessionTabsSyncState>
|
||||
|
||||
expect(patch.tabsByWorktree).toBeDefined()
|
||||
expect(Object.hasOwn(patch.tabsByWorktree!, WT)).toBe(false)
|
||||
})
|
||||
|
||||
// Why: a runtime that has published nothing for a worktree still answers a forced snapshot with a
|
||||
// synthesized empty frame. That is "ask me later", not "the user emptied this".
|
||||
it('does not tombstone from a synthesized unpublished frame', () => {
|
||||
const mirrored = applyWebSessionTabsSnapshot(
|
||||
makeState(),
|
||||
hostTerminalSnapshot(),
|
||||
ENV,
|
||||
NOW
|
||||
) as Partial<WebSessionTabsSyncState>
|
||||
const mirroredTabs = mirrored.tabsByWorktree?.[WT] as TerminalTab[]
|
||||
|
||||
const patch = applyWebSessionTabsSnapshot(
|
||||
makeState({ tabsByWorktree: { [WT]: mirroredTabs } }),
|
||||
emptySnapshot({
|
||||
publicationEpoch: UNPUBLISHED_WORKTREE_PUBLICATION_EPOCH,
|
||||
snapshotVersion: 0
|
||||
}),
|
||||
ENV,
|
||||
NOW
|
||||
) as Partial<WebSessionTabsSyncState>
|
||||
|
||||
expect(patch.tabsByWorktree).toBeDefined()
|
||||
expect(Object.hasOwn(patch.tabsByWorktree!, WT)).toBe(false)
|
||||
})
|
||||
})
|
||||
@@ -14,6 +14,11 @@ import {
|
||||
makeSnapshot,
|
||||
resetWebSessionTabsSyncTestState
|
||||
} from './web-session-tabs-sync-test-harness'
|
||||
import {
|
||||
beginWebRuntimeInitialTerminalBootstrap,
|
||||
endWebRuntimeInitialTerminalBootstrap,
|
||||
isWebRuntimeInitialTerminalBootstrapInFlight
|
||||
} from './web-runtime-initial-terminal-bootstrap'
|
||||
|
||||
vi.mock('../store', () => ({
|
||||
useAppStore: {
|
||||
@@ -55,7 +60,8 @@ describe('applyWebSessionTabsSnapshot', () => {
|
||||
activeWorktreeId: WT,
|
||||
requestedInitialTerminal: false,
|
||||
snapshotIsFresh: staleIsFresh,
|
||||
localTerminalCount: 0
|
||||
localTerminalCount: 0,
|
||||
hasPersistedTerminalState: false
|
||||
})
|
||||
).toBe(false)
|
||||
})
|
||||
@@ -73,11 +79,83 @@ describe('applyWebSessionTabsSnapshot', () => {
|
||||
activeWorktreeId: WT,
|
||||
requestedInitialTerminal: false,
|
||||
snapshotIsFresh: true,
|
||||
localTerminalCount: 1
|
||||
localTerminalCount: 1,
|
||||
hasPersistedTerminalState: true
|
||||
})
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
// Why: the workspace the user emptied on purpose. STA-6173 — the mirror used to delete the row,
|
||||
// which reads back as "never initialized", so every focus of a runtime-owned workspace seeded a
|
||||
// terminal the local path had long since stopped seeding.
|
||||
it('does not bootstrap a terminal when an explicit empty row records the closed last terminal', () => {
|
||||
const freshEmpty = makeSnapshot([], {
|
||||
activeGroupId: null,
|
||||
activeTabId: null,
|
||||
activeTabType: null
|
||||
})
|
||||
|
||||
expect(
|
||||
shouldBootstrapInitialWebRuntimeTerminal({
|
||||
event: { type: 'snapshot', ...freshEmpty },
|
||||
activeWorktreeId: WT,
|
||||
requestedInitialTerminal: false,
|
||||
snapshotIsFresh: true,
|
||||
localTerminalCount: 0,
|
||||
hasPersistedTerminalState: true
|
||||
})
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('bootstraps a terminal for a workspace that has no terminal row at all', () => {
|
||||
const freshEmpty = makeSnapshot([], {
|
||||
activeGroupId: null,
|
||||
activeTabId: null,
|
||||
activeTabType: null
|
||||
})
|
||||
|
||||
expect(
|
||||
shouldBootstrapInitialWebRuntimeTerminal({
|
||||
event: { type: 'snapshot', ...freshEmpty },
|
||||
activeWorktreeId: WT,
|
||||
requestedInitialTerminal: false,
|
||||
snapshotIsFresh: true,
|
||||
localTerminalCount: 0,
|
||||
hasPersistedTerminalState: false
|
||||
})
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
// Why: the second half of STA-6173. One focus re-runs the subscription effect (environment,
|
||||
// connection generation and session-ready all settle during a workspace switch), and the old
|
||||
// closure-local flag re-armed with it, so both closures seeded before either create mirrored.
|
||||
it('declines a second bootstrap while one is already in flight for the worktree', () => {
|
||||
const freshEmpty = makeSnapshot([], {
|
||||
activeGroupId: null,
|
||||
activeTabId: null,
|
||||
activeTabType: null
|
||||
})
|
||||
const decide = (): boolean =>
|
||||
shouldBootstrapInitialWebRuntimeTerminal({
|
||||
event: { type: 'snapshot', ...freshEmpty },
|
||||
activeWorktreeId: WT,
|
||||
// What a freshly installed closure passes: its own flag is false, so only the shared latch
|
||||
// can stop it.
|
||||
requestedInitialTerminal: isWebRuntimeInitialTerminalBootstrapInFlight(WT),
|
||||
snapshotIsFresh: true,
|
||||
localTerminalCount: 0,
|
||||
hasPersistedTerminalState: false
|
||||
})
|
||||
|
||||
expect(decide()).toBe(true)
|
||||
expect(beginWebRuntimeInitialTerminalBootstrap(WT)).toBe(true)
|
||||
expect(beginWebRuntimeInitialTerminalBootstrap(WT)).toBe(false)
|
||||
expect(decide()).toBe(false)
|
||||
|
||||
endWebRuntimeInitialTerminalBootstrap(WT)
|
||||
expect(decide()).toBe(true)
|
||||
})
|
||||
|
||||
it('does not respawn after wake when activation already requested a respawn', () => {
|
||||
const freshEmpty = makeSnapshot([], {
|
||||
activeGroupId: null,
|
||||
|
||||
@@ -4,6 +4,7 @@ import { resetWebSessionFocusIntentForTests } from './web-session-focus-intent'
|
||||
import { resetWebSessionCloseIntentForTests } from './web-session-close-intent'
|
||||
import { resetWebSessionReorderIntentForTests } from './web-session-reorder-intent'
|
||||
import { resetWebAgentSessionHandoffsForTests } from './web-agent-session-handoff'
|
||||
import { resetWebRuntimeInitialTerminalBootstrapForTests } from './web-runtime-initial-terminal-bootstrap'
|
||||
import {
|
||||
resetWebSessionTabsSnapshotFreshnessForTests,
|
||||
type WebSessionTabsSyncState
|
||||
@@ -24,6 +25,7 @@ export function resetWebSessionTabsSyncTestState(): void {
|
||||
resetWebSessionCloseIntentForTests()
|
||||
resetWebSessionReorderIntentForTests()
|
||||
resetWebAgentSessionHandoffsForTests()
|
||||
resetWebRuntimeInitialTerminalBootstrapForTests()
|
||||
}
|
||||
|
||||
export function layoutHasGroup(layout: TabGroupLayoutNode | undefined, groupId: string): boolean {
|
||||
|
||||
@@ -36,6 +36,11 @@ import {
|
||||
shouldSkipWebRuntimeWakeTerminalRespawn
|
||||
} from '../web-runtime-wake-terminal-respawn'
|
||||
import { createWebRuntimeSessionTerminal } from '../web-runtime-session'
|
||||
import {
|
||||
beginWebRuntimeInitialTerminalBootstrap,
|
||||
endWebRuntimeInitialTerminalBootstrap,
|
||||
isWebRuntimeInitialTerminalBootstrapInFlight
|
||||
} from '../web-runtime-initial-terminal-bootstrap'
|
||||
import { toRuntimeWorktreeSelector } from '../runtime-worktree-selector'
|
||||
import type { SessionTabsStreamEvent } from './state'
|
||||
|
||||
@@ -143,9 +148,15 @@ export function installActiveSessionTabsSubscription({
|
||||
const bootstrap = shouldBootstrapInitialWebRuntimeTerminal({
|
||||
event: recoveredEvent,
|
||||
activeWorktreeId,
|
||||
requestedInitialTerminal,
|
||||
// Why both: the closure flag keeps one failed attempt from retrying on every later frame of
|
||||
// the same subscription, and the shared latch is what survives the effect re-runs a workspace
|
||||
// switch triggers — without it a second closure seeds a second terminal while the first
|
||||
// create is still in flight (STA-6173).
|
||||
requestedInitialTerminal:
|
||||
requestedInitialTerminal || isWebRuntimeInitialTerminalBootstrapInFlight(activeWorktreeId),
|
||||
snapshotIsFresh: decision.apply,
|
||||
localTerminalCount
|
||||
localTerminalCount,
|
||||
hasPersistedTerminalState: Object.hasOwn(syncState.tabsByWorktree, activeWorktreeId)
|
||||
})
|
||||
const respawn = shouldRespawnWebRuntimeTerminalAfterWake({
|
||||
event: recoveredEvent,
|
||||
@@ -185,13 +196,13 @@ export function installActiveSessionTabsSubscription({
|
||||
visibilitySnapshotAccepted.current(environmentId, recovered, receivedFrame, runtimeId)
|
||||
}
|
||||
try {
|
||||
if (isCurrent() && bootstrap) {
|
||||
if (isCurrent() && bootstrap && beginWebRuntimeInitialTerminalBootstrap(activeWorktreeId)) {
|
||||
requestedInitialTerminal = true
|
||||
await createWebRuntimeSessionTerminal({
|
||||
worktreeId: activeWorktreeId,
|
||||
environmentId,
|
||||
activate: true
|
||||
})
|
||||
}).finally(() => endWebRuntimeInitialTerminalBootstrap(activeWorktreeId))
|
||||
} else if (isCurrent() && respawn && beginWebRuntimeWakeTerminalRespawn(activeWorktreeId)) {
|
||||
requestedRespawnAfterWake = true
|
||||
await createWebRuntimeSessionTerminal({
|
||||
|
||||
@@ -27,6 +27,8 @@ import {
|
||||
shouldReplaceTerminalTab
|
||||
} from './terminal-surfaces'
|
||||
import { buildMirroredTerminalTabs } from './terminal-build'
|
||||
import { isWebSessionTabsWorktreeRemovalFrame } from './session-tabs-inventory-absence'
|
||||
import { hostSnapshotAffirmsWorktreeContents } from '../host-session-snapshot-authority'
|
||||
|
||||
export function prepareWebSessionTabsSnapshotBase(
|
||||
state: WebSessionTabsSyncState,
|
||||
@@ -163,10 +165,19 @@ export function prepareWebSessionTabsSnapshotBase(
|
||||
)
|
||||
const mirroredTerminalTabEntries = mirroredTerminalTabs.map((entry) => entry.tab)
|
||||
const retainedTerminalIds = new Set(retainedTerminalTabs.map((tab) => tab.id))
|
||||
// Why an empty row rather than a deleted key: an explicit empty row is the closed-last-terminal
|
||||
// tombstone every seeder reads (initial-terminal.ts), and dropping the key reads back as "never
|
||||
// initialized", which re-seeds the workspace on every focus (STA-6173). `sameTerminalTabs` treats
|
||||
// a missing row and an empty one as equal, so a worktree that never had a terminal still gets no
|
||||
// row. A removal frame really is gone, and a synthesized unpublished frame is "ask me later", not
|
||||
// evidence the user emptied anything — neither may leave a tombstone behind.
|
||||
const nextTerminalTabs =
|
||||
retainedTerminalTabs.length + mirroredTerminalTabEntries.length > 0
|
||||
? [...retainedTerminalTabs, ...mirroredTerminalTabEntries]
|
||||
: null
|
||||
: isWebSessionTabsWorktreeRemovalFrame(snapshot) ||
|
||||
!hostSnapshotAffirmsWorktreeContents(snapshot)
|
||||
? null
|
||||
: []
|
||||
const mirroredTerminalIds = new Set(mirroredTerminalTabEntries.map((tab) => tab.id))
|
||||
const removedTerminalIds = new Set(
|
||||
currentTerminalTabs.filter((tab) => !retainedTerminalIds.has(tab.id)).map((tab) => tab.id)
|
||||
|
||||
@@ -21,6 +21,7 @@ import {
|
||||
} from './tracking'
|
||||
import { clearWebSessionTabsTrackingForWorktree } from './tracking-lifecycle'
|
||||
import { queueAcceptedWebSessionTerminalSnapshot } from '../web-session-terminal-handle-events'
|
||||
import { shouldAutoCreateInitialTerminal } from '@/components/terminal/initial-terminal'
|
||||
|
||||
/** A frame's fate, paired with whether that fate is host evidence for the worktree. */
|
||||
export type WebSessionTabsSnapshotDecision = {
|
||||
@@ -135,12 +136,16 @@ export function shouldBootstrapInitialWebRuntimeTerminal(args: {
|
||||
requestedInitialTerminal: boolean
|
||||
snapshotIsFresh: boolean
|
||||
localTerminalCount: number
|
||||
hasPersistedTerminalState: boolean
|
||||
}): boolean {
|
||||
return (
|
||||
args.snapshotIsFresh &&
|
||||
args.event.type === 'snapshot' &&
|
||||
args.event.tabs.length === 0 &&
|
||||
args.localTerminalCount === 0 &&
|
||||
// Why the shared predicate: the host owning the terminals does not change what an empty
|
||||
// workspace means. A missing row is "never initialized", an explicit empty row is "the user
|
||||
// closed the last terminal", and only the local seeder used to read the difference (STA-6173).
|
||||
shouldAutoCreateInitialTerminal(args.localTerminalCount, args.hasPersistedTerminalState) &&
|
||||
!args.requestedInitialTerminal &&
|
||||
args.activeWorktreeId === args.event.worktree
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user