From eff410fe3031a5526bee75f8d9ca3be3b6e9a8f3 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Sun, 11 Oct 2026 01:48:35 -0400 Subject: [PATCH] refactor(mobile-relay): drop unread freshness fields from server workspace lists The phone reads only the rows, so stale, fetchedAt, totalCount and truncated were dead wire weight. The desktop still serves a server's last rows while the server is unreachable. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- ...ile-relay-host-catalog.integration.test.ts | 17 ++--- .../mobile-relay-host-catalog.test.ts | 72 ++++++------------- .../mobile-relay-host-catalog.ts | 48 ++++--------- src/shared/mobile-relay-hosts-contract.ts | 14 +--- 4 files changed, 45 insertions(+), 106 deletions(-) diff --git a/src/main/runtime/mobile-desktop-relay/mobile-relay-host-catalog.integration.test.ts b/src/main/runtime/mobile-desktop-relay/mobile-relay-host-catalog.integration.test.ts index 14e4d72f2ca..1f2645ae13d 100644 --- a/src/main/runtime/mobile-desktop-relay/mobile-relay-host-catalog.integration.test.ts +++ b/src/main/runtime/mobile-desktop-relay/mobile-relay-host-catalog.integration.test.ts @@ -155,7 +155,7 @@ describe('mobile relay hosts: phone -> desktop lists the servers it shows', () = return { box, contact, phoneCall } } - it('lists rows live through the desktop, then its last rows as stale once a server is unreachable', async () => { + it('lists rows live through the desktop, then its last rows once a server is unreachable', async () => { const { box, contact, phoneCall } = await startTopology() await expect(phoneCall('mobileRelay.hosts.list')).resolves.toEqual({ @@ -168,17 +168,14 @@ describe('mobile relay hosts: phone -> desktop lists the servers it shows', () = expect(warm).toMatchObject({ worktrees: [ { worktreeId: 'repo-1::/tmp/worktree-a', hostId: 'runtime:box', branch: 'feature/foo' } - ], - totalCount: 1, - stale: false + ] }) // The update-needed server still answers the desktop, so its rows are live. await expect( phoneCall('mobileRelay.hosts.worktrees', { hostId: 'runtime:old' }) ).resolves.toMatchObject({ - worktrees: [{ worktreeId: 'repo-1::/tmp/worktree-a', hostId: 'runtime:old' }], - stale: false + worktrees: [{ worktreeId: 'repo-1::/tmp/worktree-a', hostId: 'runtime:old' }] }) await box.stop() @@ -186,12 +183,10 @@ describe('mobile relay hosts: phone -> desktop lists the servers it shows', () = await expect(phoneCall('mobileRelay.hosts.list')).resolves.toMatchObject({ hosts: [{ hostId: 'runtime:box', relay: 'unavailable' }, { hostId: 'runtime:old' }] }) - const asRecord = z.record(z.string(), z.unknown()) - const offline = asRecord.parse( - await phoneCall('mobileRelay.hosts.worktrees', { hostId: 'runtime:box' }) - ) // The last rows exactly as fetched, provenance kept. - expect(offline).toEqual({ ...asRecord.parse(warm), stale: true }) + await expect( + phoneCall('mobileRelay.hosts.worktrees', { hostId: 'runtime:box' }) + ).resolves.toEqual(warm) }) it('answers an empty list on a desktop that relays nowhere', async () => { diff --git a/src/main/runtime/mobile-desktop-relay/mobile-relay-host-catalog.test.ts b/src/main/runtime/mobile-desktop-relay/mobile-relay-host-catalog.test.ts index 059e3b4b56e..8759eee02ca 100644 --- a/src/main/runtime/mobile-desktop-relay/mobile-relay-host-catalog.test.ts +++ b/src/main/runtime/mobile-desktop-relay/mobile-relay-host-catalog.test.ts @@ -99,17 +99,7 @@ function fakeHosts( } function catalog(hosts: MobileDesktopRelayHosts, labels = new Map()) { - let now = 1_000 - return { - catalog: new MobileRelayHostCatalog({ - hosts, - hostLabelOverrides: () => labels, - now: () => now - }), - advance: (ms: number) => { - now += ms - } - } + return { catalog: new MobileRelayHostCatalog({ hosts, hostLabelOverrides: () => labels }) } } describe('mobile relay host catalog', () => { @@ -205,11 +195,7 @@ describe('mobile relay host catalog', () => { { worktreeId: 'w-local', hostId: 'runtime:env', status: 'working' }, { worktreeId: 'w-ssh', hostId: 'runtime:env', status: 'inactive' }, { worktreeId: 'w-legacy', hostId: 'runtime:env', status: 'done' } - ], - totalCount: 3, - truncated: false, - fetchedAt: 1_000, - stale: false + ] }) expect(call).toHaveBeenCalledWith( expect.objectContaining({ environmentId: 'env' }), @@ -220,34 +206,27 @@ describe('mobile relay host catalog', () => { ) }) - it('keeps the last rows, unchanged and marked stale, once the server stops answering', async () => { + it('keeps the last rows, unchanged, once the server stops answering', async () => { const { hosts, call, state } = fakeHosts([ { id: 'env', name: 'Box', snapshot: snapshot('env', 'live') } ]) call.mockResolvedValue(psReply([{ worktreeId: 'w', hostId: 'local', status: 'working' }])) - const { catalog: hostCatalog, advance } = catalog(hosts) + const { catalog: hostCatalog } = catalog(hosts) await hostCatalog.worktrees('runtime:env') - advance(5_000) state.environments = [{ id: 'env', name: 'Box', snapshot: snapshot('env', 'unreachable') }] const offline = await hostCatalog.worktrees('runtime:env') // Never re-fetched while the desktop itself cannot reach it, and never rewritten as idle. expect(call).toHaveBeenCalledTimes(1) expect(offline).toEqual({ - worktrees: [{ worktreeId: 'w', hostId: 'runtime:env', status: 'working' }], - totalCount: 1, - truncated: false, - fetchedAt: 1_000, - stale: true + worktrees: [{ worktreeId: 'w', hostId: 'runtime:env', status: 'working' }] }) - // A failed fetch on a server that looks reachable also serves the last rows as stale. + // A failed fetch on a server that looks reachable also serves the last rows. state.environments = [{ id: 'env', name: 'Box', snapshot: snapshot('env', 'live') }] call.mockRejectedValueOnce(new Error('socket closed')) - await expect(hostCatalog.worktrees('runtime:env')).resolves.toMatchObject({ - fetchedAt: 1_000, - stale: true - }) + await expect(hostCatalog.worktrees('runtime:env')).resolves.toEqual(offline) + expect(call).toHaveBeenCalledTimes(2) }) it('omits rows mid-delete, as a phone asking the server itself would never see them', async () => { @@ -267,10 +246,8 @@ describe('mobile relay host catalog', () => { }, _meta: { runtimeId: 'runtime-a' } }) - await expect(catalog(hosts).catalog.worktrees('runtime:env')).resolves.toMatchObject({ - worktrees: [{ worktreeId: 'kept', hostId: 'runtime:env' }], - totalCount: 4, - truncated: true + await expect(catalog(hosts).catalog.worktrees('runtime:env')).resolves.toEqual({ + worktrees: [{ worktreeId: 'kept', hostId: 'runtime:env' }] }) }) @@ -279,9 +256,8 @@ describe('mobile relay host catalog', () => { { id: 'old', name: 'ThinkPad', snapshot: snapshot('old', 'live', false) } ]) call.mockResolvedValue(psReply([{ worktreeId: 'w', hostId: 'local' }])) - await expect(catalog(hosts).catalog.worktrees('runtime:old')).resolves.toMatchObject({ - worktrees: [{ worktreeId: 'w', hostId: 'runtime:old' }], - stale: false + await expect(catalog(hosts).catalog.worktrees('runtime:old')).resolves.toEqual({ + worktrees: [{ worktreeId: 'w', hostId: 'runtime:old' }] }) }) @@ -345,7 +321,7 @@ describe('mobile relay host catalog', () => { await expect(hostCatalog.worktrees('runtime:env')).resolves.toEqual({ worktrees: null }) }) - it('keeps the last rows, stale, for a server manually disconnected but still configured', async () => { + it('keeps the last rows for a server manually disconnected but still configured', async () => { const { hosts, call, state } = fakeHosts([ { id: 'env', name: 'Box', snapshot: snapshot('env', 'live') } ]) @@ -361,9 +337,8 @@ describe('mobile relay host catalog', () => { } ] state.retire('env') - await expect(hostCatalog.worktrees('runtime:env')).resolves.toMatchObject({ - worktrees: [{ worktreeId: 'w', hostId: 'runtime:env' }], - stale: true + await expect(hostCatalog.worktrees('runtime:env')).resolves.toEqual({ + worktrees: [{ worktreeId: 'w', hostId: 'runtime:env' }] }) expect(call).toHaveBeenCalledTimes(1) }) @@ -374,8 +349,7 @@ describe('mobile relay host catalog', () => { ]) call.mockResolvedValue(psReply([{ worktreeId: 'w', hostId: 'local' }])) await expect(catalog(hosts).catalog.worktrees('runtime:env')).resolves.toMatchObject({ - worktrees: [{ worktreeId: 'w' }], - stale: false + worktrees: [{ worktreeId: 'w' }] }) expect(call).toHaveBeenCalledWith(expect.anything(), 'worktree.ps', expect.anything(), { timeoutMs: 5_000, @@ -383,7 +357,7 @@ describe('mobile relay host catalog', () => { }) }) - it('answers with the last rows, marked stale, when the server never answers the fetch', async () => { + it('answers with the last rows when the server never answers the fetch', async () => { vi.useFakeTimers() try { const { hosts, call } = fakeHosts([ @@ -398,10 +372,8 @@ describe('mobile relay host catalog', () => { await vi.advanceTimersByTimeAsync(4_999) expect(settled).toBe(false) await vi.advanceTimersByTimeAsync(1) - await expect(rows).resolves.toMatchObject({ - worktrees: [{ worktreeId: 'w', hostId: 'runtime:env' }], - fetchedAt: 1_000, - stale: true + await expect(rows).resolves.toEqual({ + worktrees: [{ worktreeId: 'w', hostId: 'runtime:env' }] }) } finally { vi.useRealTimers() @@ -419,10 +391,8 @@ describe('mobile relay host catalog', () => { const second = hostCatalog.worktrees('runtime:env') await vi.waitFor(() => expect(call).toHaveBeenCalledTimes(1)) answerFetch(psReply([{ worktreeId: 'w', hostId: 'local' }])) - await expect(Promise.all([first, second])).resolves.toMatchObject([ - { stale: false }, - { stale: false } - ]) + const rows = { worktrees: [{ worktreeId: 'w', hostId: 'runtime:env' }] } + await expect(Promise.all([first, second])).resolves.toEqual([rows, rows]) expect(call).toHaveBeenCalledTimes(1) }) }) diff --git a/src/main/runtime/mobile-desktop-relay/mobile-relay-host-catalog.ts b/src/main/runtime/mobile-desktop-relay/mobile-relay-host-catalog.ts index 0a87f12f64c..90fb0a5bab3 100644 --- a/src/main/runtime/mobile-desktop-relay/mobile-relay-host-catalog.ts +++ b/src/main/runtime/mobile-desktop-relay/mobile-relay-host-catalog.ts @@ -26,18 +26,11 @@ const SERVER_WORKTREE_LIMIT = 10_000 // Why well under the phone's 30s request timeout: a hung server must answer with its last rows. const SERVER_FETCH_TIMEOUT_MS = 5_000 -const WorktreePsReplySchema = z.object({ - worktrees: z.array(z.looseObject({})), - totalCount: z.number(), - truncated: z.boolean() -}) +const WorktreePsReplySchema = z.object({ worktrees: z.array(z.looseObject({})) }) type CachedServerWorktrees = { fence: string - fetchedAt: number worktrees: MobileRelayServerWorktreeRow[] - totalCount: number - truncated: boolean } type DescribedHost = MobileRelayHost & { @@ -54,13 +47,12 @@ type DescribedHost = MobileRelayHost & { export class MobileRelayHostCatalog { private readonly cached = new Map() // Keyed by server identity, so a poll after a re-pair never joins the old server's fetch. - private readonly refreshing = new Map>() + private readonly refreshing = new Map>() constructor( private readonly options: { hosts: MobileDesktopRelayHosts hostLabelOverrides: () => ReadonlyMap - now?: () => number } ) {} @@ -84,7 +76,9 @@ export class MobileRelayHostCatalog { if (!host) { return { worktrees: null } } - const refreshed = host.health === 'available' && (await withinFetchBound(this.refresh(host))) + if (host.health === 'available') { + await withinFetchBound(this.refresh(host)) + } const cached = this.cached.get(host.environmentId) const current = this.options.hosts .list() @@ -94,14 +88,7 @@ export class MobileRelayHostCatalog { return { worktrees: null } } // Why: the desktop asks with background-removal support; a phone asking itself never sees these rows. - const worktrees = cached.worktrees.filter((row) => row.removing !== true) - return { - worktrees, - totalCount: cached.totalCount - (cached.worktrees.length - worktrees.length), - truncated: cached.truncated, - fetchedAt: cached.fetchedAt, - stale: !refreshed - } + return { worktrees: cached.worktrees.filter((row) => row.removing !== true) } } private describe(): DescribedHost[] { @@ -146,7 +133,7 @@ export class MobileRelayHostCatalog { }) } - private refresh(host: DescribedHost): Promise { + private refresh(host: DescribedHost): Promise { const key = `${host.environmentId}\0${fenceOf(host.identity)}` const inFlight = this.refreshing.get(key) if (inFlight) { @@ -157,7 +144,7 @@ export class MobileRelayHostCatalog { return refresh } - private async fetchAsDesktop(host: DescribedHost): Promise { + private async fetchAsDesktop(host: DescribedHost): Promise { try { const response = await this.options.hosts.call( host, @@ -168,20 +155,15 @@ export class MobileRelayHostCatalog { ) const reply = response.ok ? WorktreePsReplySchema.safeParse(response.result) : null if (!reply?.success) { - return false + return } this.cached.set(host.environmentId, { // Why the identity taken before the fetch: the call's own check cannot see a re-pair after dispatch. fence: fenceOf(host.identity), - fetchedAt: (this.options.now ?? Date.now)(), - worktrees: stampServerWorktreeRows(host.environmentId, reply.data.worktrees), - totalCount: reply.data.totalCount, - truncated: reply.data.truncated + worktrees: stampServerWorktreeRows(host.environmentId, reply.data.worktrees) }) - return true } catch { - // An unreachable server keeps its last rows; they are served as stale. - return false + // An unreachable server keeps its last rows. } } } @@ -190,11 +172,11 @@ function fenceOf(identity: DescribedHost['identity']): string { return `${identity.pairingRevision}\0${identity.runtimeId}` } -/** False once the bound passes; the call's own timeout starts only after the desktop's per-server queue. */ -async function withinFetchBound(refresh: Promise): Promise { +/** Settles once the bound passes; the call's own timeout starts only after the desktop's per-server queue. */ +async function withinFetchBound(refresh: Promise): Promise { let timer: ReturnType | undefined - const bound = new Promise((resolve) => { - timer = setTimeout(() => resolve(false), SERVER_FETCH_TIMEOUT_MS) + const bound = new Promise((resolve) => { + timer = setTimeout(resolve, SERVER_FETCH_TIMEOUT_MS) }) try { return await Promise.race([refresh, bound]) diff --git a/src/shared/mobile-relay-hosts-contract.ts b/src/shared/mobile-relay-hosts-contract.ts index 11defa4482a..975108e2dc5 100644 --- a/src/shared/mobile-relay-hosts-contract.ts +++ b/src/shared/mobile-relay-hosts-contract.ts @@ -25,15 +25,7 @@ export const MobileRelayHostWorktreesParamsSchema = z export type MobileRelayServerWorktreeRow = Record & { hostId: `runtime:${string}` } /** - * The server's own `worktree.ps` rows as the desktop last fetched them. `stale` means this call - * could not refresh them; rows are never rewritten beyond their host id. + * The server's own `worktree.ps` rows as the desktop last fetched them, refreshed when the server + * answers; rows are never rewritten beyond their host id. */ -export type MobileRelayHostWorktreesResult = - | { - worktrees: MobileRelayServerWorktreeRow[] - totalCount: number - truncated: boolean - fetchedAt: number - stale: boolean - } - | { worktrees: null } +export type MobileRelayHostWorktreesResult = { worktrees: MobileRelayServerWorktreeRow[] | null }