From d15ac6aa19da7eef2ea34eef3108646213ecaacf Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 6 Aug 2026 18:45:35 -0400 Subject: [PATCH 1/2] ship: checkpoint remote shell swap fix --- .../components/terminals/SessionListPane.tsx | 2 + .../renderer/state/crossMachineLanes.test.ts | 112 ++++++++++++++++ .../src/renderer/state/crossMachineLanes.ts | 126 ++++++++++++++---- .../features/terminals-and-sessions/README.md | 8 +- 4 files changed, 220 insertions(+), 28 deletions(-) diff --git a/apps/desktop/src/renderer/components/terminals/SessionListPane.tsx b/apps/desktop/src/renderer/components/terminals/SessionListPane.tsx index 8fb001901..025795eb8 100644 --- a/apps/desktop/src/renderer/components/terminals/SessionListPane.tsx +++ b/apps/desktop/src/renderer/components/terminals/SessionListPane.tsx @@ -1009,6 +1009,8 @@ export const SessionListPane = React.memo(function SessionListPane({ const { foreignRows, markersByLaneId } = useCrossMachineLaneUnion( crossMachineSyncActive, allSessionsUnfiltered, + workLaneSortMode, + workLaneOrder, ); const [createLaneOpen, setCreateLaneOpen] = useState(false); const [settleUndo, setSettleUndo] = useState<{ ids: string[]; count: number } | null>(null); diff --git a/apps/desktop/src/renderer/state/crossMachineLanes.test.ts b/apps/desktop/src/renderer/state/crossMachineLanes.test.ts index 17b664707..36971d7c4 100644 --- a/apps/desktop/src/renderer/state/crossMachineLanes.test.ts +++ b/apps/desktop/src/renderer/state/crossMachineLanes.test.ts @@ -174,6 +174,47 @@ describe("offline machines stay in the sidebar, dimmed", () => { .toEqual(["lane-online", "lane-offline"]); }); + it("uses stable Work ordering unless activity mode is selected", () => { + const rows = buildCrossMachineLaneRows({ + localLanes: [], + machines: { + "target-studio": { + machineId: "target-studio", + machineName: "Mac Studio (12)", + targetId: "target-studio", + projectId: "project-a", + online: true, + lanes: [ + makeLane({ id: "lane-old", name: "Old", createdAt: "2026-07-20T10:00:00.000Z" }), + makeLane({ id: "lane-new", name: "New", createdAt: "2026-07-21T10:00:00.000Z" }), + ], + sessions: [ + makeSession({ + id: "session-old", + laneId: "lane-old", + lastActivityAt: "2026-07-30T10:00:00.000Z", + }), + makeSession({ + id: "session-new", + laneId: "lane-new", + lastActivityAt: "2026-07-29T10:00:00.000Z", + }), + ], + prs: [], + lastSyncedAtMs: Date.now(), + error: null, + }, + }, + }); + + expect(orderCrossMachineRows(rows).map((row) => row.lane.id)) + .toEqual(["lane-new", "lane-old"]); + expect(orderCrossMachineRows(rows, "activity").map((row) => row.lane.id)) + .toEqual(["lane-old", "lane-new"]); + expect(orderCrossMachineRows([...rows].reverse()).map((row) => row.lane.id)) + .toEqual(["lane-new", "lane-old"]); + }); + it("forgets a machine outright only when asked to", () => { useAppStore.getState().mergeCrossMachineLanes({ machineId: "target-studio", @@ -1201,6 +1242,77 @@ describe("cross-machine refresh scheduling", () => { stop(); }); + it("does not restart the foreign poll for every active session event", async () => { + vi.useFakeTimers(); + let emitSessionChange: (() => void) | null = null; + let emitLaneChange: (() => void) | null = null; + const callAction = vi.fn(async ( + _targetId: string, + _projectId: string, + request: { domain: string; action: string }, + ) => ({ + result: request.domain === "lane" + ? { lanes: [] } + : request.domain === "pr" + ? { prs: [] } + : { sessions: [] }, + })); + window.ade = { + sessions: { + onChanged: vi.fn((listener: () => void) => { + emitSessionChange = listener; + return () => { emitSessionChange = null; }; + }), + }, + lanes: { + onLifecycleEvent: vi.fn((listener: () => void) => { + emitLaneChange = listener; + return () => { emitLaneChange = null; }; + }), + }, + remoteRuntime: { + callAction, + getConnectionSnapshot: vi.fn(async () => ({ + connections: [{ + state: "connected", + target: { id: "target-studio", name: "Mac Studio (12)", hostname: "studio" }, + projects: [{ + projectId: "project-a", + rootPath: "/repo-a", + displayName: "Repo A", + gitOriginUrl: "git@github.com:acme/repo-a.git", + }], + }], + connectedCount: 1, + })), + onConnectionSnapshotChanged: vi.fn(() => () => {}), + }, + } as unknown as typeof window.ade; + + const stop = startCrossMachineLaneSync({ + scopeKey: "local:/repo-a", + repoDisplayName: "Repo A", + repoOriginUrl: "git@github.com:acme/repo-a.git", + boundTargetId: null, + boundProjectId: null, + }); + await Promise.resolve(); + await vi.advanceTimersByTimeAsync(400); + const initialCallCount = callAction.mock.calls.length; + expect(initialCallCount).toBe(3); + + emitSessionChange?.(); + emitLaneChange?.(); + emitSessionChange?.(); + await vi.advanceTimersByTimeAsync(2_000); + expect(callAction).toHaveBeenCalledTimes(initialCallCount); + + await vi.advanceTimersByTimeAsync(8_500); + expect(callAction.mock.calls.length).toBeGreaterThan(initialCallCount); + + stop(); + }); + it("re-reads lanes on their own slow cadence, and immediately for an unseen lane", async () => { vi.useFakeTimers(); const requests: Array<{ domain: string; action: string }> = []; diff --git a/apps/desktop/src/renderer/state/crossMachineLanes.ts b/apps/desktop/src/renderer/state/crossMachineLanes.ts index ec5a9f63f..aeba9c27d 100644 --- a/apps/desktop/src/renderer/state/crossMachineLanes.ts +++ b/apps/desktop/src/renderer/state/crossMachineLanes.ts @@ -18,11 +18,12 @@ * leave the sidebar for two reasons only: the machine is gone from the * registry, or it has been unreachable for a full day. * - * Performance shape: active-binding refreshes are event-driven. Other machines - * do not have a renderer change feed, so one shared, ref-counted fallback - * refresh keeps them current while the window is visible, and stops entirely - * while it is not. Foreign reads are bounded, timed out, - * generation-cancellable, and never gate the local list. + * Performance shape: the active binding has its own event-driven refresh. Other + * machines do not have a renderer change feed, so one shared, ref-counted + * fallback refresh keeps them current while the window is visible. Change + * events can request that refresh, but never pull a foreign read ahead of its + * bounded cadence. Reads are timed out, generation-cancellable, and never gate + * the local list. */ import { useEffect, useMemo, useRef, useState } from "react"; @@ -53,6 +54,11 @@ import { deriveLaneMachineOptions, type LaneMachineOption, } from "../components/lanes/laneMachines"; +import { + compareWorkLanes, + type WorkLaneOrderInput, + type WorkLaneSortMode, +} from "../components/terminals/workLaneOrder"; import { rootAppStoreApi, selectActiveProjectStateKey, @@ -95,6 +101,7 @@ const FOREIGN_LANE_REFRESH_MS = 30_000; * actually looked at, which is when a stale badge would be visible. */ type LaneReadDepth = "identity" | "status"; +const EMPTY_WORK_LANE_ORDER: readonly string[] = []; const OFFLINE_DIVERGENCE_MAX_AGE_MS = 60_000; /** * Floor on how long a drop must persist before Work shows a machine as offline. @@ -364,13 +371,27 @@ function normalizeBranchRef(branchRef: string | null | undefined): string { return trimmed.replace(/^refs\/heads\//, ""); } -function laneActivityRank(row: CrossMachineLaneRow): number { - let latest = Date.parse(row.lane.createdAt ?? ""); +function laneLastActivityMs(row: CrossMachineLaneRow): number | null { + let latest: number | null = null; for (const session of row.sessions) { const at = Date.parse(session.lastActivityAt ?? session.startedAt ?? ""); - if (Number.isFinite(at) && at > (Number.isFinite(latest) ? latest : -Infinity)) latest = at; + if (!Number.isNaN(at) && (latest === null || at > latest)) latest = at; } - return Number.isFinite(latest) ? latest : 0; + return latest; +} + +function crossMachineLaneOrderInput(row: CrossMachineLaneRow): WorkLaneOrderInput { + return { + id: row.lane.id, + name: row.lane.name, + laneType: row.lane.laneType, + createdAt: row.lane.createdAt, + lastActivityMs: laneLastActivityMs(row), + // Foreign pin/shelving state is renderer-local and is applied after this + // sync-layer order. Keeping these false preserves that layer boundary. + quiet: false, + pinned: false, + }; } /** @@ -478,18 +499,35 @@ export function buildCrossMachineLaneRows(input: { } /** - * Sidebar order for foreign rows: reachable machines first, each group by most - * recent activity. A dropped machine's lanes are still worth seeing — that is - * the point of dimming rather than hiding — but they are not what you are about - * to act on, so they sink below the live ones instead of interleaving with them. + * Sidebar order for foreign rows: reachable machines first, then the same raw + * Work sort mode used by the local lane list. Foreign shelving and pin state + * are applied later by SessionListPane, so this comparator deliberately keeps + * those presentation-only concerns out of the sync layer. + * + * Activity is therefore opt-in. The default is creation order, which prevents + * terminal output (`lastActivityAt`) from moving a shell between two renders. + * The final machine/lane key makes equal sort values total across refreshes. */ export function orderCrossMachineRows( rows: readonly CrossMachineLaneRow[], + mode: WorkLaneSortMode = "created", + manualOrder: readonly string[] = EMPTY_WORK_LANE_ORDER, ): CrossMachineLaneRow[] { - return [...rows].sort((left, right) => { - if (left.online !== right.online) return left.online ? -1 : 1; - return laneActivityRank(right) - laneActivityRank(left); + const manualIndex = new Map(); + manualOrder.forEach((id, index) => { + if (!manualIndex.has(id)) manualIndex.set(id, index); }); + return rows + .map((row) => ({ row, input: crossMachineLaneOrderInput(row) })) + .sort((left, right) => { + if (left.row.online !== right.row.online) return left.row.online ? -1 : 1; + const modeDelta = compareWorkLanes(left.input, right.input, mode, manualIndex); + if (modeDelta !== 0) return modeDelta; + return `${left.row.machineId}:${left.row.lane.id}`.localeCompare( + `${right.row.machineId}:${right.row.lane.id}`, + ); + }) + .map(({ row }) => row); } /** @@ -734,6 +772,9 @@ type SyncRuntime = { refreshTimer: ReturnType | null; refreshInFlight: boolean; refreshQueued: boolean; + refreshQueuedStatus: boolean; + /** Start time of the most recent foreign refresh, used to rate-limit events. */ + lastRefreshStartedAtMs: number | null; /** Open drop record per machine that is currently not connected. */ dropsByMachineId: Map; /** Re-evaluates reachability when the next drop deadline lapses. */ @@ -792,6 +833,8 @@ const runtime: SyncRuntime = { refreshTimer: null, refreshInFlight: false, refreshQueued: false, + refreshQueuedStatus: false, + lastRefreshStartedAtMs: null, dropsByMachineId: new Map(), graceTimer: null, laneReadAtMsByMachineId: new Map(), @@ -1311,23 +1354,42 @@ async function runRefresh(): Promise { * @param depth `status` for the triggers that justify paying for git status — * the surface being mounted, retargeted, or looked at again, a machine * appearing, and the poll timer coming round. Change feeds leave it at - * `identity`; see {@link LaneReadDepth}. + * `identity`; see {@link LaneReadDepth}. Identity events coalesce onto the + * existing foreign cadence instead of restarting it. */ function scheduleRefresh(depth: LaneReadDepth = "identity"): void { - if (depth === "status") runtime.pendingLaneReadDepth = "status"; + const force = depth === "status"; + if (force) runtime.pendingLaneReadDepth = "status"; + // Hidden windows read nothing at all. The visibility listener in `attach` + // calls straight back here on the way in, so the list is refreshed once, + // immediately, when it can actually be seen again. + if (!isDocumentVisible()) { + if (force && runtime.refreshTimer) { + clearTimeout(runtime.refreshTimer); + runtime.refreshTimer = null; + } + return; + } + // A change feed is not a reason to pull the foreign poll forward. The active + // machine still updates through its own event path; the union's foreign reads + // stay bounded even when shell output produces frequent session events. + if (runtime.refreshTimer && !force) return; if (runtime.refreshTimer) { clearTimeout(runtime.refreshTimer); runtime.refreshTimer = null; } - // Hidden windows read nothing at all. The visibility listener in `attach` - // calls straight back here on the way in, so the list is refreshed once, - // immediately, when it can actually be seen again. - if (!isDocumentVisible()) return; if (runtime.refreshInFlight) { runtime.refreshQueued = true; + runtime.refreshQueuedStatus ||= force; return; } if (runtime.timer) return; + const elapsedSinceLastRefresh = runtime.lastRefreshStartedAtMs == null + ? null + : Date.now() - runtime.lastRefreshStartedAtMs; + const delay = force || elapsedSinceLastRefresh == null + ? REFRESH_COALESCE_MS + : Math.max(REFRESH_COALESCE_MS, FOREIGN_MACHINE_REFRESH_MS - elapsedSinceLastRefresh); runtime.timer = setTimeout(() => { runtime.timer = null; // A refresh outlives its own runtime: reads are bounded but slow, and @@ -1337,6 +1399,7 @@ function scheduleRefresh(depth: LaneReadDepth = "identity"): void { // schedules anything at all. const lifecycle = runtime.lifecycle; runtime.refreshInFlight = true; + runtime.lastRefreshStartedAtMs = Date.now(); void runRefresh() .catch(() => {}) .finally(() => { @@ -1344,8 +1407,10 @@ function scheduleRefresh(depth: LaneReadDepth = "identity"): void { runtime.refreshInFlight = false; if (runtime.refCount === 0) return; if (runtime.refreshQueued) { + const queuedDepth: LaneReadDepth = runtime.refreshQueuedStatus ? "status" : "identity"; runtime.refreshQueued = false; - scheduleRefresh(); + runtime.refreshQueuedStatus = false; + scheduleRefresh(queuedDepth); return; } if (!isDocumentVisible()) return; @@ -1357,7 +1422,7 @@ function scheduleRefresh(depth: LaneReadDepth = "identity"): void { scheduleRefresh("status"); }, FOREIGN_MACHINE_REFRESH_MS); }); - }, REFRESH_COALESCE_MS); + }, delay); } /** @@ -1634,10 +1699,11 @@ function attach(): void { // shared bounded refresh below because preload exposes no per-target push // subscription. // - // Both are change feeds, so both refresh at `identity` depth. On the web + // Both are change feeds, so both request `identity` depth. On the web // transport a lifecycle event is synthesized from a coarse table invalidation, // and reading status here would write the rows that produce the next - // invalidation — the loop this depth split exists to cut. + // invalidation — the loop this depth split exists to cut. The scheduler also + // keeps these events from restarting the foreign poll on every shell tick. const unsubscribeSessions = window.ade?.sessions?.onChanged?.(() => scheduleRefresh()); if (unsubscribeSessions) runtime.disposers.push(unsubscribeSessions); const unsubscribeLanes = window.ade?.lanes?.onLifecycleEvent?.(() => scheduleRefresh()); @@ -1692,7 +1758,9 @@ function detach(): void { } resetMachineTracking(); runtime.refreshQueued = false; + runtime.refreshQueuedStatus = false; runtime.refreshInFlight = false; + runtime.lastRefreshStartedAtMs = null; runtime.pendingLaneReadDepth = "identity"; for (const dispose of runtime.disposers.splice(0)) { try { @@ -1769,6 +1837,8 @@ export function resetCrossMachineLaneSyncForTest(): void { export function useCrossMachineLaneUnion( active = true, localSessions?: readonly TerminalSessionSummary[], + workLaneSortMode: WorkLaneSortMode = "created", + workLaneOrder: readonly string[] = EMPTY_WORK_LANE_ORDER, ): CrossMachineUnion { // Stabilized by CONTENT, not by the array's identity. The caller's roster is // replaced wholesale by every session poll (~5s while anything is running), @@ -1915,6 +1985,8 @@ export function useCrossMachineLaneUnion( // computed correctly and then thrown away one line later. const foreignRows = orderCrossMachineRows( rows.filter((row) => !row.isActiveBinding), + workLaneSortMode, + workLaneOrder, ); // Single-machine setups take this branch forever: no marker map is built and // the lane header renders exactly as it did before this feature existed. @@ -1927,5 +1999,5 @@ export function useCrossMachineLaneUnion( foreignRows, markersByLaneId: resolveCrossMachineLaneMarkers(rows), }; - }, [rows]); + }, [rows, workLaneOrder, workLaneSortMode]); } diff --git a/docs/features/terminals-and-sessions/README.md b/docs/features/terminals-and-sessions/README.md index 624302c23..d629cddec 100644 --- a/docs/features/terminals-and-sessions/README.md +++ b/docs/features/terminals-and-sessions/README.md @@ -739,7 +739,13 @@ Renderer surfaces: the session names. The hook stabilizes that id set by content rather than by the roster array's identity, because the roster is replaced wholesale by every session poll and keying on it would rebuild every foreign row, marker, and - ordering on a timer. + ordering on a timer. Foreign rows stay grouped by reachability, then use the + same selected raw Work sort mode as local lanes; Created is the default, so + live PTY output does not reshuffle shell rows, while Activity remains an + explicit choice. Equal sort values fall back to the owning machine/lane key. + Session and lane change feeds request an identity refresh but coalesce onto + the shared visible-window cadence instead of restarting a remote read for + every output tick. Its marker resolver separately distinguishes `isActiveBinding` (where a lane renders) from `isThisMachine` (whether it is marked): a remote-bound tab still marks all lanes that are elsewhere, even when it has no foreign union rows. From 58678fa86179f4729b9f8d11b2a207eb8e4ae6c7 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 6 Aug 2026 19:17:44 -0400 Subject: [PATCH 2/2] =?UTF-8?q?ship:=20iteration=201=20=E2=80=94=20fix=20c?= =?UTF-8?q?omposite=20foreign=20ordering=20and=20typecheck?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../renderer/state/crossMachineLanes.test.ts | 61 ++++++++++++++++--- .../src/renderer/state/crossMachineLanes.ts | 10 +-- 2 files changed, 58 insertions(+), 13 deletions(-) diff --git a/apps/desktop/src/renderer/state/crossMachineLanes.test.ts b/apps/desktop/src/renderer/state/crossMachineLanes.test.ts index 36971d7c4..e9165a057 100644 --- a/apps/desktop/src/renderer/state/crossMachineLanes.test.ts +++ b/apps/desktop/src/renderer/state/crossMachineLanes.test.ts @@ -215,6 +215,47 @@ describe("offline machines stay in the sidebar, dimmed", () => { .toEqual(["lane-new", "lane-old"]); }); + it("uses composite machine/lane keys for shared foreign manual order", () => { + const rows = buildCrossMachineLaneRows({ + localLanes: [], + machines: { + "target-studio": { + machineId: "target-studio", + machineName: "Mac Studio (12)", + targetId: "target-studio", + projectId: "project-a", + online: true, + lanes: [makeLane({ id: "shared-lane", name: "Studio Shared" })], + sessions: [], + prs: [], + lastSyncedAtMs: Date.now(), + error: null, + }, + "target-laptop": { + machineId: "target-laptop", + machineName: "MacBook Pro (97)", + targetId: "target-laptop", + projectId: "project-a", + online: true, + lanes: [makeLane({ id: "shared-lane", name: "Laptop Shared" })], + sessions: [], + prs: [], + lastSyncedAtMs: Date.now(), + error: null, + }, + }, + }); + const manualOrder = ["target-laptop:shared-lane", "target-studio:shared-lane"]; + const toCompositeIds = (ordered: readonly typeof rows[number][]) => + ordered.map((row) => `${row.machineId}:${row.lane.id}`); + + expect(toCompositeIds(orderCrossMachineRows(rows, "manual", manualOrder))) + .toEqual(manualOrder); + expect(toCompositeIds(orderCrossMachineRows([...rows].reverse(), "manual", manualOrder))) + .toEqual(manualOrder); + expect(new Set(toCompositeIds(rows))).toEqual(new Set(manualOrder)); + }); + it("forgets a machine outright only when asked to", () => { useAppStore.getState().mergeCrossMachineLanes({ machineId: "target-studio", @@ -1244,8 +1285,10 @@ describe("cross-machine refresh scheduling", () => { it("does not restart the foreign poll for every active session event", async () => { vi.useFakeTimers(); - let emitSessionChange: (() => void) | null = null; - let emitLaneChange: (() => void) | null = null; + const emitters: { + session: (() => void) | null; + lane: (() => void) | null; + } = { session: null, lane: null }; const callAction = vi.fn(async ( _targetId: string, _projectId: string, @@ -1260,14 +1303,14 @@ describe("cross-machine refresh scheduling", () => { window.ade = { sessions: { onChanged: vi.fn((listener: () => void) => { - emitSessionChange = listener; - return () => { emitSessionChange = null; }; + emitters.session = listener; + return () => { emitters.session = null; }; }), }, lanes: { onLifecycleEvent: vi.fn((listener: () => void) => { - emitLaneChange = listener; - return () => { emitLaneChange = null; }; + emitters.lane = listener; + return () => { emitters.lane = null; }; }), }, remoteRuntime: { @@ -1301,9 +1344,9 @@ describe("cross-machine refresh scheduling", () => { const initialCallCount = callAction.mock.calls.length; expect(initialCallCount).toBe(3); - emitSessionChange?.(); - emitLaneChange?.(); - emitSessionChange?.(); + emitters.session?.(); + emitters.lane?.(); + emitters.session?.(); await vi.advanceTimersByTimeAsync(2_000); expect(callAction).toHaveBeenCalledTimes(initialCallCount); diff --git a/apps/desktop/src/renderer/state/crossMachineLanes.ts b/apps/desktop/src/renderer/state/crossMachineLanes.ts index aeba9c27d..067a95d40 100644 --- a/apps/desktop/src/renderer/state/crossMachineLanes.ts +++ b/apps/desktop/src/renderer/state/crossMachineLanes.ts @@ -380,9 +380,13 @@ function laneLastActivityMs(row: CrossMachineLaneRow): number | null { return latest; } +function crossMachineLaneId(row: CrossMachineLaneRow): string { + return `${row.machineId}:${row.lane.id}`; +} + function crossMachineLaneOrderInput(row: CrossMachineLaneRow): WorkLaneOrderInput { return { - id: row.lane.id, + id: crossMachineLaneId(row), name: row.lane.name, laneType: row.lane.laneType, createdAt: row.lane.createdAt, @@ -523,9 +527,7 @@ export function orderCrossMachineRows( if (left.row.online !== right.row.online) return left.row.online ? -1 : 1; const modeDelta = compareWorkLanes(left.input, right.input, mode, manualIndex); if (modeDelta !== 0) return modeDelta; - return `${left.row.machineId}:${left.row.lane.id}`.localeCompare( - `${right.row.machineId}:${right.row.lane.id}`, - ); + return crossMachineLaneId(left.row).localeCompare(crossMachineLaneId(right.row)); }) .map(({ row }) => row); }