Sidebar: land the switch-on fixes from the flip branch on main, switch still off (#31466)

* fix(sidebar): land flip-a's switch-on fixes on main, still dark (no flip)

The fixes found while the sidebar ran from main's feed, without turning
that on: ORCA_SIDEBAR_INPUTS_IS_MAIN stays false, and none of flip-a's
stop-gaps come along.

- Main stops counting a closed tab's terminal once it was asked to stop
  and no pane shows it; a failed or refused stop takes the request back,
  and a host out of contact stays unverifiable.
- Main fetches a paired server's rows with the transport's normal bound,
  so a server slower than 5 s still lists.
- A deferred unread raise flips the folder row it was decided for, and a
  read while the owner check is out cancels it.
- Phone: no worktree.ps flash before the feed; a host's last installed
  feed stands in while a fresh read is out; it highlights its own
  last-opened workspace and a fold never scrolls; projects file under
  their project group with the desktop's indent; the collapse spinner
  shows only after 500 ms; "Reopen to update" keeps the rows on screen.
- e2e helpers and journeys for the switch-on suite.

* fix(sidebar): keep an owed relay stop marked, clear a thrown stop, and prune pending unread raises

- A stop-and-wait that throws takes its stop request back, like one that fails.
- A final stop the legacy relay still owes stays marked, since the relay retries it.
- Pending unread raises are tracked only while one waits, so the map no longer grows.
- The paired-server fallback poll test gives its third fetch a reply and asserts live.
This commit is contained in:
Jinwoo Hong
2026-10-21 12:24:27 -04:00
committed by GitHub
parent 50b07c0e8f
commit 92bbb380a6
53 changed files with 2404 additions and 299 deletions
@@ -239,7 +239,7 @@ describe('mobile relay host catalog', () => {
'worktree.ps',
// The phone's full-catalog limit: the server's default page would silently truncate.
{ limit: 10_000, supportsWorktreeVisibilitySourceDefaults: true },
{ timeoutMs: 5_000, expected: { pairingRevision: 1, runtimeId: 'runtime-a' } }
{ expected: { pairingRevision: 1, runtimeId: 'runtime-a' } }
)
})
@@ -330,7 +330,6 @@ describe('mobile relay host catalog', () => {
const next = hostCatalog.worktrees('runtime:env')
await vi.waitFor(() => expect(call).toHaveBeenCalledTimes(2))
expect(call).toHaveBeenLastCalledWith(expect.anything(), 'worktree.ps', expect.anything(), {
timeoutMs: 5_000,
expected: { pairingRevision: 2, runtimeId: 'runtime-a' }
})
await expect(next).resolves.toMatchObject({ worktrees: [{ worktreeId: 'new-server-row' }] })
@@ -395,7 +394,6 @@ describe('mobile relay host catalog', () => {
worktrees: [{ worktreeId: 'w' }]
})
expect(call).toHaveBeenCalledWith(expect.anything(), 'worktree.ps', expect.anything(), {
timeoutMs: 5_000,
expected: { pairingRevision: 1, runtimeId: null }
})
})
@@ -0,0 +1,179 @@
// Runs on plain Node, as orcad does: main's "Hide sleeping" retention reads the runtime's own facts.
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { OrcaRuntimeService } from '../orca-runtime'
import type { RuntimeStore } from '../runtime-store-contract'
import type { PersistedUIState } from '../../../shared/persisted-ui-state-types'
import type { ResolvedWorktreeSnapshot } from '../runtime-resolved-worktree-cache'
import type { ResolvedWorktree } from '../runtime-worktree-path-identity'
vi.mock('../../../shared/sidebar-inputs-switch', () => ({ ORCA_SIDEBAR_INPUTS_IS_MAIN: true }))
const repo = { id: 'repo-1', path: '/r', displayName: 'r', badgeColor: 'blue', addedAt: 1 }
const WT = 'repo-1::/r/a'
const LEAF = '22222222-2222-4222-8222-222222222222'
function worktree(id: string): ResolvedWorktree {
return {
id,
repoId: repo.id,
path: id,
branch: 'a',
head: '',
isBare: false,
isMainWorktree: false,
displayName: id,
comment: '',
linkedIssue: null,
linkedPR: null,
linkedLinearIssue: null,
linkedGitLabMR: null,
linkedGitLabIssue: null,
isArchived: false,
isUnread: false,
isPinned: false,
sortOrder: 0,
lastActivityAt: 0,
parentWorktreeId: null,
childWorktreeIds: [],
lineage: null,
git: { path: id, head: '', branch: 'a', isBare: false, isMainWorktree: false }
}
}
class Probe extends OrcaRuntimeService {
protected override async listResolvedWorktreeSnapshot(): Promise<ResolvedWorktreeSnapshot> {
return { worktrees: [worktree(WT)], platformByRepoId: new Map() }
}
}
type Host = NonNullable<OrcaRuntimeService['sidebarFacts']>
/** The window's graph: one terminal tab in WT, running pty-1. */
function publishTerminalTab(runtime: Host): void {
runtime.syncWindowGraph(1, {
tabs: [{ tabId: 'tab-1', worktreeId: WT, title: 'Terminal', activeLeafId: LEAF, layout: null }],
leaves: [{ tabId: 'tab-1', worktreeId: WT, leafId: LEAF, paneRuntimeId: 1, ptyId: 'pty-1' }],
mobileSessionTabs: [
{
worktree: WT,
publicationEpoch: 'renderer:test',
snapshotVersion: 1,
activeGroupId: null,
activeTabId: null,
activeTabType: null,
tabs: [
{
type: 'terminal',
id: `tab-1::${LEAF}`,
parentTabId: 'tab-1',
leafId: LEAF,
title: 'Terminal',
ptyId: 'pty-1',
isActive: true
}
]
}
]
})
}
/** A window with one terminal open in WT, its list mounted, "Hide sleeping" on. */
async function createHost(): Promise<Host> {
const store: Pick<RuntimeStore, 'getRepos' | 'getUI'> = {
getRepos: () => [repo],
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: retention reads only hideSleepingWorkspaces.
getUI: () => ({ hideSleepingWorkspaces: true }) as PersistedUIState
}
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: these paths touch only repos and the UI flag.
const runtime = new Probe(store as RuntimeStore).sidebarFacts
if (!runtime) {
throw new Error('built without the sidebar facts layer')
}
publishTerminalTab(runtime)
runtime.beginSidebarListSession('session-1')
await vi.advanceTimersByTimeAsync(300)
expect(runtime.getWorkspaceActivityFacts().facts[WT]?.terminal).toBe('live')
return runtime
}
const retained = (runtime: Host): readonly string[] =>
runtime.getSidebarTemporalSnapshot().retention?.retainedIds ?? []
/** The window's graph once its terminal tab in WT is closed. */
function publishNoTabs(runtime: Host): void {
runtime.syncWindowGraph(1, {
tabs: [],
leaves: [],
mobileSessionTabs: [
{
worktree: WT,
publicationEpoch: 'renderer:test',
snapshotVersion: 2,
activeGroupId: null,
activeTabId: null,
activeTabType: null,
tabs: []
}
]
})
}
// pty-1 is recorded as the spawn records it, so it reads live until its exit arrives.
async function createHostWithSpawnedPty(): Promise<Host> {
const runtime = await createHost()
runtime.registerPty('pty-1', WT)
return runtime
}
describe('main sleeping retention and a closed last tab', () => {
beforeEach(() => {
vi.useFakeTimers()
})
afterEach(() => {
vi.useRealTimers()
})
it('drops the row as the window closes the tab, before the terminal exits', async () => {
const runtime = await createHostWithSpawnedPty()
// As closeTab does: pty:kill first, then the graph without the tab.
runtime.markPtyStopRequested('pty-1')
expect(runtime.getWorkspaceActivityFacts().facts[WT]?.terminal).toBe('live')
publishNoTabs(runtime)
expect(runtime.getWorkspaceActivityFacts().facts[WT]).toBeUndefined()
expect(retained(runtime)).toEqual([])
})
it('drops the row when the stop request lands after the graph', async () => {
const runtime = await createHostWithSpawnedPty()
publishNoTabs(runtime)
expect(runtime.getWorkspaceActivityFacts().facts[WT]?.terminal).toBe('live')
runtime.markPtyStopRequested('pty-1')
expect(runtime.getWorkspaceActivityFacts().facts[WT]).toBeUndefined()
expect(retained(runtime)).toEqual([])
})
it('keeps a terminal live while a pane still shows it, though asked to stop', async () => {
const runtime = await createHostWithSpawnedPty()
runtime.markPtyStopRequested('pty-1')
await vi.advanceTimersByTimeAsync(300)
expect(runtime.getWorkspaceActivityFacts().facts[WT]?.terminal).toBe('live')
})
it('keeps a closed tab’s terminal unverifiable when its host stops answering', async () => {
const runtime = await createHostWithSpawnedPty()
runtime.markPtyStopRequested('pty-1')
publishNoTabs(runtime)
runtime.markPtyLivenessUnverifiable('pty-1', 'ssh contact lost')
expect(runtime.getWorkspaceActivityFacts().facts[WT]?.terminal).toBe('unverifiable')
})
it('counts the terminal live again when its stop fails', async () => {
const runtime = await createHostWithSpawnedPty()
runtime.markPtyStopRequested('pty-1')
publishNoTabs(runtime)
expect(runtime.getWorkspaceActivityFacts().facts[WT]).toBeUndefined()
runtime.clearPtyStopRequest('pty-1')
expect(runtime.getWorkspaceActivityFacts().facts[WT]?.terminal).toBe('live')
})
})
@@ -33,6 +33,7 @@ export abstract class OrcaRuntimeWithWorkspaceActivityFacts extends OrcaRuntimeW
override syncWindowGraph(...args: Parameters<Runtime['syncWindowGraph']>) {
const previousTabs = this.tabs
const previousLeaves = this.leaves
const result = super.syncWindowGraph(...args)
// Only a tab moving workspace can change where an agent row is attributed.
const moved = [...this.tabs.values()].some(
@@ -41,9 +42,22 @@ export abstract class OrcaRuntimeWithWorkspaceActivityFacts extends OrcaRuntimeW
if (moved || [...previousTabs.keys()].some((tabId) => !this.tabs.has(tabId))) {
this.workspaceActivityFacts.resyncAgentPanes()
}
// Now, not on the session publication: a closed pane's stopping PTY stops counting here.
// syncWindowGraph replaces this.leaves (session-window-graph.ts), so previousLeaves stays intact.
this.refreshWorkspaces(leafPtyChangedWorktrees(previousLeaves, this.leaves))
return result
}
override markPtyStopRequested(ptyId: string): void {
super.markPtyStopRequested(ptyId)
this.refreshWorkspaces(this.ptyWorktreeIds(ptyId))
}
override clearPtyStopRequest(ptyId: string): void {
super.clearPtyStopRequest(ptyId)
this.refreshWorkspaces(this.ptyWorktreeIds(ptyId))
}
protected override notifyMobileSessionTabsChangedNow(
worktreeId: string,
changeSequence: number
@@ -102,6 +116,11 @@ export abstract class OrcaRuntimeWithWorkspaceActivityFacts extends OrcaRuntimeW
}
}
// Asked to stop with no pane left: the window that closed it already shows it gone.
private isClosing(ptyId: string): boolean {
return this.isPtyStopRequested(ptyId) && this.getLeavesForPty(ptyId).length === 0
}
private ptyWorktreeIds(ptyId: string): Set<string> {
const worktreeIds = new Set(this.getLeavesForPty(ptyId).map((leaf) => leaf.worktreeId))
const recorded = this.ptysById.get(ptyId)?.worktreeId
@@ -123,8 +142,8 @@ export abstract class OrcaRuntimeWithWorkspaceActivityFacts extends OrcaRuntimeW
}
}
// getLivePtyIdsForWorktree's identity rule in one pass. A host that only lost contact keeps its
// terminals unverifiable, never gone, even while the record still reads connected.
// Matches every host form of the worktree's id. A host that only lost contact keeps its terminals
// unverifiable, never gone, even while the record still reads connected or is being closed.
private readWorkspaceTerminalLiveness(worktreeId: string): WorkspaceTerminalLiveness | null {
const key = worktreeIdentityKey(worktreeId)
const owns = (id: string): boolean => id === worktreeId || worktreeIdentityKey(id) === key
@@ -136,7 +155,7 @@ export abstract class OrcaRuntimeWithWorkspaceActivityFacts extends OrcaRuntimeW
}
if (this.getPtyLivenessVerdict(surface.ptyId)?.status === 'unverifiable') {
unverifiable = true
} else if (surface.connected) {
} else if (surface.connected && !this.isClosing(surface.ptyId)) {
return 'live'
}
}
@@ -144,3 +163,23 @@ export abstract class OrcaRuntimeWithWorkspaceActivityFacts extends OrcaRuntimeW
return unverifiable ? 'unverifiable' : null
}
}
function leafPtyChangedWorktrees(
before: ReadonlyMap<string, { worktreeId: string; ptyId: string | null }>,
after: ReadonlyMap<string, { worktreeId: string; ptyId: string | null }>
): Set<string> {
const changed = new Set<string>()
for (const [leafId, leaf] of before) {
const next = after.get(leafId)
if (next?.ptyId !== leaf.ptyId || next.worktreeId !== leaf.worktreeId) {
changed.add(leaf.worktreeId)
}
}
for (const [leafId, leaf] of after) {
const previous = before.get(leafId)
if (previous?.ptyId !== leaf.ptyId || previous.worktreeId !== leaf.worktreeId) {
changed.add(leaf.worktreeId)
}
}
return changed
}
@@ -124,6 +124,9 @@ function fakeHosts(servers: { id: string; live: boolean }[]) {
}
}
// callRuntimeEnvironment's bound for a call that names none.
const DEFAULT_TRANSPORT_TIMEOUT_MS = 15_000
const GROUP = {
name: 'school',
parentPath: null,
@@ -172,7 +175,7 @@ describe('PairedServerRowsProducer', () => {
{ environmentId: 'box' },
'worktree.ps',
{ limit: 10_000, supportsWorktreeVisibilitySourceDefaults: true },
{ timeoutMs: 5_000, expected: { pairingRevision: 1, runtimeId: 'runtime-a' } }
{ expected: { pairingRevision: 1, runtimeId: 'runtime-a' } }
)
expect(rowsOf(producer, 'runtime:box')).toEqual({
hostId: 'runtime:box',
@@ -239,6 +242,90 @@ describe('PairedServerRowsProducer', () => {
expect(rowsOf(producer, 'runtime:box')?.rows.map((entry) => entry.worktreeId)).toEqual(['new'])
})
// A server answering after `delayMs`, or failing as the transport would once its bound passes.
function slowServer(call: ReturnType<typeof fakeHosts>['call'], delayMs: number, rows: string[]) {
call.mockImplementationOnce(
(_host, _method, _params, options) =>
new Promise((resolve, reject) => {
const boundMs = options?.timeoutMs ?? DEFAULT_TRANSPORT_TIMEOUT_MS
if (delayMs > boundMs) {
setTimeout(() => reject(new Error('timed out')), boundMs)
} else {
setTimeout(() => resolve(psReply(rows.map((id) => row(id)))), delayMs)
}
})
)
}
it('lists a server that takes 8 s to answer', async () => {
const { hosts, call, watches } = fakeHosts([{ id: 'box', live: true }])
slowServer(call, 8_000, ['a'])
const producer = new PairedServerRowsProducer({
hasDemand: () => true,
hostLabelOverrides: () => new Map()
})
producer.attach(hosts)
watches.get('box')?.onChange()
await vi.advanceTimersByTimeAsync(7_999)
expect(rowsOf(producer, 'runtime:box')?.rows).toEqual([])
await vi.advanceTimersByTimeAsync(1)
expect(rowsOf(producer, 'runtime:box')).toMatchObject({
freshness: 'live',
rows: [{ worktreeId: 'a' }]
})
})
it('holds one fetch per server, and notices during it coalesce into one follow-up', async () => {
const { hosts, call, watches } = fakeHosts([{ id: 'box', live: true }])
slowServer(call, 8_000, ['old'])
slowServer(call, 8_000, ['new'])
const producer = new PairedServerRowsProducer({
hasDemand: () => true,
hostLabelOverrides: () => new Map()
})
producer.attach(hosts)
watches.get('box')?.onChange()
await vi.advanceTimersByTimeAsync(1_000)
// Change notices and a reader's refresh arrive while it runs.
watches.get('box')?.onChange()
watches.get('box')?.onChange()
void producer.refresh('box')
await vi.advanceTimersByTimeAsync(6_000)
expect(call).toHaveBeenCalledTimes(1)
await vi.advanceTimersByTimeAsync(1_000)
expect(call).toHaveBeenCalledTimes(2)
// The fallback poll answers too, so the rows stay live.
call.mockResolvedValueOnce(psReply([row('new')]))
await vi.advanceTimersByTimeAsync(30_000)
expect(call).toHaveBeenCalledTimes(3)
expect(rowsOf(producer, 'runtime:box')).toMatchObject({ freshness: 'live' })
expect(rowsOf(producer, 'runtime:box')?.rows.map((entry) => entry.worktreeId)).toEqual(['new'])
})
it('keeps the last rows when a fetch times out', async () => {
const { hosts, call, watches } = fakeHosts([{ id: 'box', live: true }])
call.mockResolvedValueOnce(psReply([row('a')]))
const producer = new PairedServerRowsProducer({
hasDemand: () => true,
hostLabelOverrides: () => new Map()
})
producer.attach(hosts)
watches.get('box')?.onChange()
await vi.advanceTimersByTimeAsync(0)
slowServer(call, 60_000, ['never'])
watches.get('box')?.onChange()
// The transport's normal bound, not the phone's 5 s, ends the wait.
await vi.advanceTimersByTimeAsync(DEFAULT_TRANSPORT_TIMEOUT_MS - 1)
expect(rowsOf(producer, 'runtime:box')?.freshness).toBe('live')
await vi.advanceTimersByTimeAsync(1)
expect(rowsOf(producer, 'runtime:box')).toMatchObject({
freshness: 'unverifiable',
rows: [{ worktreeId: 'a' }]
})
})
it('keeps the last rows as unverifiable when the server loses contact or stops answering', async () => {
const { hosts, call, watches, state, publishStatus } = fakeHosts([{ id: 'box', live: true }])
call.mockResolvedValueOnce(psReply([row('a')]))
@@ -17,7 +17,6 @@ import {
} from '../mobile-desktop-relay/mobile-relay-host-descriptions'
import type { ServerWorktreeRows } from '../mobile-desktop-relay/mobile-relay-host-catalog'
import {
SERVER_FETCH_TIMEOUT_MS,
SERVER_WORKTREE_LIMIT,
WorktreePsReplySchema,
stampServerWorktreeRows
@@ -244,8 +243,8 @@ export class PairedServerRowsProducer implements ServerWorktreeRows {
const { contactGeneration } = entry
const call = async (method: string, params?: unknown) =>
hosts.call({ environmentId: entry.environmentId }, method, params, {
// Why the transport bound too: it ends the hung call, so the next request fetches afresh.
timeoutMs: SERVER_FETCH_TIMEOUT_MS,
// Why the transport's normal bound: nothing waits on this fetch (readers take the latest
// rows), so a slow server's answer must still land; the bound still ends a hung call.
expected: entry.identity
})
// Only on contact, ready, a replay or reposChanged, which every catalog write announces.
@@ -53,6 +53,11 @@ export abstract class OrcaRuntimeWithMarkPtyLivenessUnverifiable extends OrcaRun
this.stopRequestedPtyIds.add(ptyId)
}
/** A stop that failed or was refused: the PTY runs on, so nothing may read it as closing. */
clearPtyStopRequest(ptyId: string): void {
this.stopRequestedPtyIds.delete(ptyId)
}
isPtyStopRequested(ptyId: string): boolean {
return this.stopRequestedPtyIds.has(ptyId)
}