Harden layout validation, watcher lifecycle, and connection robustness (#7924)
* Harden layout validation, watcher lifecycle, and connection robustness - Throw instead of silently skipping when the packaged daemon-entry is missing, preventing layout regressions from passing build checks. - Terminate idle parcel-watcher processes to reclaim native handles and avoid crash-prone native node module teardowns on shutdown. - Bind the persisted WS fallback port first to prevent orphaning active mobile pairings when the preferred port becomes free again. - Cap concurrent disk reads for restored dirty tab verification at three to prevent startup connection bottlenecks on remote SSH workspaces. * Queue file IDs instead of snapshots in restored conflict scans This avoids using stale file snapshots (e.g., outdated disk signatures) if a tab is saved, closed, or re-baselined while waiting in the queue behind the concurrency limit. The live state is now fetched from the store and validated immediately before initiating the disk read.
This commit is contained in:
parent
73f1bbfc94
commit
69befad13e
|
|
@ -2,7 +2,10 @@ const { chmodSync, existsSync, readdirSync } = require('node:fs')
|
|||
const { execFileSync } = require('node:child_process')
|
||||
const { join, resolve } = require('node:path')
|
||||
const electronBuilderNativeRebuild = require('./scripts/electron-builder-native-rebuild.cjs')
|
||||
const { verifyPackagedDaemonEntryBoots } = require('./scripts/verify-packaged-daemon-entry.cjs')
|
||||
const {
|
||||
assertPackagedDaemonEntryExists,
|
||||
verifyPackagedDaemonEntryBoots
|
||||
} = require('./scripts/verify-packaged-daemon-entry.cjs')
|
||||
const {
|
||||
createPackagedRuntimeNodeModuleResources,
|
||||
prunePackagedRuntimeNodeModules,
|
||||
|
|
@ -141,8 +144,12 @@ module.exports = {
|
|||
if (context.arch === hostArchEnum || context.arch === 4) {
|
||||
verifyPackagedDaemonEntryBoots(resourcesDir)
|
||||
} else {
|
||||
// Why: a cross-arch slice can't be booted by the host Node, but the
|
||||
// unpacked entry must still exist — its absence is a layout regression
|
||||
// regardless of arch, so only the boot is skipped, not the check.
|
||||
assertPackagedDaemonEntryExists(resourcesDir)
|
||||
console.log(
|
||||
`[verify-packaged-daemon-entry] skipped cross-arch slice (target ${context.arch}, host ${process.arch})`
|
||||
`[verify-packaged-daemon-entry] skipped boot on cross-arch slice (target ${context.arch}, host ${process.arch})`
|
||||
)
|
||||
}
|
||||
chmodUnixCliLaunchers(resourcesDir, context.electronPlatformName)
|
||||
|
|
|
|||
|
|
@ -353,6 +353,15 @@ describe('electron-builder config', () => {
|
|||
const launcherPath = join(resourcesDir, 'bin', 'orca-ide')
|
||||
await mkdir(join(resourcesDir, 'bin'), { recursive: true })
|
||||
await mkdir(join(resourcesDir, 'node_modules', 'zod', 'src'), { recursive: true })
|
||||
// Why: afterPack now fails hard when the unpacked daemon entry is
|
||||
// missing, so the fixture must carry one like a real package layout.
|
||||
const unpackedMainDir = join(resourcesDir, 'app.asar.unpacked', 'out', 'main')
|
||||
await mkdir(unpackedMainDir, { recursive: true })
|
||||
await writeFile(
|
||||
join(unpackedMainDir, 'daemon-entry.js'),
|
||||
'console.error("Usage: daemon-entry <socket>"); process.exit(1)\n',
|
||||
'utf8'
|
||||
)
|
||||
await writeFile(launcherPath, '#!/usr/bin/env bash\n', { encoding: 'utf8', mode: 0o644 })
|
||||
|
||||
await electronBuilderConfig.afterPack({
|
||||
|
|
|
|||
|
|
@ -2,6 +2,24 @@ const { existsSync } = require('node:fs')
|
|||
const { spawnSync } = require('node:child_process')
|
||||
const { join } = require('node:path')
|
||||
|
||||
// Why: `asarUnpack` in config/electron-builder.config.cjs lists
|
||||
// out/main/daemon-entry.js on every platform, and the packaged daemon fork
|
||||
// (src/main/daemon/daemon-init.ts) resolves exactly this unpacked path. A
|
||||
// missing entry means the package layout regressed, so the check throws
|
||||
// instead of skipping — a silent skip false-passed exactly the layout bug
|
||||
// this gate exists to catch.
|
||||
function assertPackagedDaemonEntryExists(resourcesDir) {
|
||||
const entryPath = join(resourcesDir, 'app.asar.unpacked', 'out', 'main', 'daemon-entry.js')
|
||||
if (!existsSync(entryPath)) {
|
||||
throw new Error(
|
||||
`[verify-packaged-daemon-entry] missing unpacked daemon entry at ${entryPath} — ` +
|
||||
`asarUnpack expects out/main/daemon-entry.js on every platform, so the packaged ` +
|
||||
`daemon cannot be forked from this layout`
|
||||
)
|
||||
}
|
||||
return entryPath
|
||||
}
|
||||
|
||||
// Why: v1.4.129-rc.1 shipped a terminal daemon that could not load (an electron
|
||||
// `require` leaked into its bundle) while every build check passed. This boots
|
||||
// the PACKAGED daemon-entry under plain Node against the asar-unpacked layout,
|
||||
|
|
@ -14,13 +32,7 @@ const { join } = require('node:path')
|
|||
// <appOutDir>/resources elsewhere). execPath defaults to the packaging Node.
|
||||
function verifyPackagedDaemonEntryBoots(resourcesDir, options = {}) {
|
||||
const execPath = options.execPath || process.execPath
|
||||
const entryPath = join(resourcesDir, 'app.asar.unpacked', 'out', 'main', 'daemon-entry.js')
|
||||
if (!existsSync(entryPath)) {
|
||||
// Why: some targets/layouts do not unpack here; skip rather than fail so
|
||||
// the hook stays safe across platforms it has not verified.
|
||||
console.log(`[verify-packaged-daemon-entry] skipped — no unpacked entry at ${entryPath}`)
|
||||
return
|
||||
}
|
||||
const entryPath = assertPackagedDaemonEntryExists(resourcesDir)
|
||||
|
||||
const result = spawnSync(execPath, [entryPath], { encoding: 'utf8', timeout: 10_000 })
|
||||
if (result.error) {
|
||||
|
|
@ -43,4 +55,4 @@ function verifyPackagedDaemonEntryBoots(resourcesDir, options = {}) {
|
|||
console.log('[verify-packaged-daemon-entry] OK — packaged daemon-entry loads under plain Node')
|
||||
}
|
||||
|
||||
module.exports = { verifyPackagedDaemonEntryBoots }
|
||||
module.exports = { assertPackagedDaemonEntryExists, verifyPackagedDaemonEntryBoots }
|
||||
|
|
|
|||
|
|
@ -0,0 +1,57 @@
|
|||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { createRequire } from 'node:module'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
|
||||
|
||||
const require = createRequire(import.meta.url)
|
||||
const {
|
||||
assertPackagedDaemonEntryExists,
|
||||
verifyPackagedDaemonEntryBoots
|
||||
} = require('./verify-packaged-daemon-entry.cjs')
|
||||
|
||||
describe('verify-packaged-daemon-entry', () => {
|
||||
let resourcesDir
|
||||
|
||||
beforeEach(() => {
|
||||
resourcesDir = mkdtempSync(join(tmpdir(), 'orca-daemon-entry-verify-'))
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(resourcesDir, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
function writePackagedEntry(source) {
|
||||
const entryDir = join(resourcesDir, 'app.asar.unpacked', 'out', 'main')
|
||||
mkdirSync(entryDir, { recursive: true })
|
||||
writeFileSync(join(entryDir, 'daemon-entry.js'), source)
|
||||
}
|
||||
|
||||
// Why: a silent skip on a missing entry false-passed exactly the packaged
|
||||
// layout regression this gate exists to catch (rc.1 daemon-load incident).
|
||||
it('throws when the unpacked daemon entry is missing', () => {
|
||||
expect(() => assertPackagedDaemonEntryExists(resourcesDir)).toThrow(
|
||||
/missing unpacked daemon entry/
|
||||
)
|
||||
expect(() => verifyPackagedDaemonEntryBoots(resourcesDir)).toThrow(
|
||||
/missing unpacked daemon entry/
|
||||
)
|
||||
})
|
||||
|
||||
it('passes when the packaged entry loads and reaches argv parsing', () => {
|
||||
writePackagedEntry('console.error("Usage: daemon-entry <socket>"); process.exit(1)\n')
|
||||
expect(() => verifyPackagedDaemonEntryBoots(resourcesDir)).not.toThrow()
|
||||
})
|
||||
|
||||
it('fails when the packaged entry cannot resolve its module graph', () => {
|
||||
writePackagedEntry('require("orca-module-that-does-not-exist")\n')
|
||||
expect(() => verifyPackagedDaemonEntryBoots(resourcesDir)).toThrow(
|
||||
/failed to load under plain Node/
|
||||
)
|
||||
})
|
||||
|
||||
it('fails when the packaged entry never reaches argv parsing', () => {
|
||||
writePackagedEntry('process.exit(0)\n')
|
||||
expect(() => verifyPackagedDaemonEntryBoots(resourcesDir)).toThrow(/did not reach argv parsing/)
|
||||
})
|
||||
})
|
||||
|
|
@ -69,8 +69,8 @@
|
|||
"build:unpack": "pnpm run build && pnpm run ensure:electron-runtime && electron-builder --config config/electron-builder.config.cjs --dir",
|
||||
"build:win": "pnpm run build:desktop && pnpm run ensure:electron-runtime && electron-builder --config config/electron-builder.config.cjs --win",
|
||||
"build:icons": "bash resources/icon-source/generate.sh",
|
||||
"build:mac": "pnpm run build:desktop && pnpm run build:computer-macos && pnpm run ensure:electron-runtime && electron-builder --config config/electron-builder.config.cjs --mac",
|
||||
"build:mac:release": "node config/scripts/verify-macos-release-env.mjs && ORCA_MAC_RELEASE=1 pnpm run build:desktop && ORCA_MAC_RELEASE=1 pnpm run build:computer-macos && pnpm run ensure:electron-runtime && ORCA_MAC_RELEASE=1 electron-builder --config config/electron-builder.config.cjs --mac",
|
||||
"build:mac": "pnpm run build:desktop && pnpm run build:computer-macos && pnpm run build:notification-status-macos && pnpm run ensure:electron-runtime && electron-builder --config config/electron-builder.config.cjs --mac",
|
||||
"build:mac:release": "node config/scripts/verify-macos-release-env.mjs && ORCA_MAC_RELEASE=1 pnpm run build:desktop && ORCA_MAC_RELEASE=1 pnpm run build:computer-macos && ORCA_MAC_RELEASE=1 pnpm run build:notification-status-macos && pnpm run ensure:electron-runtime && ORCA_MAC_RELEASE=1 electron-builder --config config/electron-builder.config.cjs --mac",
|
||||
"build:linux": "pnpm run build:desktop && pnpm run ensure:electron-runtime && electron-builder --config config/electron-builder.config.cjs --linux AppImage deb",
|
||||
"test:e2e": "pnpm run ensure:electron-runtime && npx playwright test --config tests/playwright.config.ts --project electron-headless",
|
||||
"test:e2e:floating-mobile-emulator": "pnpm run ensure:electron-runtime && npx playwright test tests/e2e/floating-mobile-emulator-tab.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1",
|
||||
|
|
|
|||
|
|
@ -2047,7 +2047,9 @@ app.whenReady().then(async () => {
|
|||
// a random OS-assigned port — breaking deterministic mobile pairing/repro
|
||||
// scripts against the dev instance. Pin the first dev instance to 6769 so
|
||||
// ws://127.0.0.1:6769 is stable; a second dev instance still falls back via
|
||||
// ws-transport's EADDRINUSE handler.
|
||||
// ws-transport's EADDRINUSE handler. Note: once an instance has ever fallen
|
||||
// back, the persisted fallback port is re-bound in preference to 6769
|
||||
// (STA-1511) until mobile-ws-fallback-port.json is removed from userData.
|
||||
const devWsPort = is.dev && !isE2E ? 6769 : undefined
|
||||
let serveOptions: ServeOptions | null = null
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -103,8 +103,13 @@ describe('subscribeViaWatcherProcess', () => {
|
|||
it('resolves unsubscribe on the child ack', async () => {
|
||||
const promise = subscribeViaWatcherProcess('/repo', vi.fn(), {})
|
||||
const child = currentChild()
|
||||
const id = ackSubscribe(child)
|
||||
const id = ackSubscribe(child, 0)
|
||||
const subscription = await promise
|
||||
// Why: hold a second subscription so this unsubscribe exercises the ack
|
||||
// path instead of the last-subscriber idle kill.
|
||||
const keepAlivePromise = subscribeViaWatcherProcess('/other', vi.fn(), {})
|
||||
ackSubscribe(child)
|
||||
await keepAlivePromise
|
||||
|
||||
const unsubPromise = subscription.unsubscribe()
|
||||
expect(child.sent.at(-1)).toEqual({ op: 'unsubscribe', id })
|
||||
|
|
@ -115,8 +120,11 @@ describe('subscribeViaWatcherProcess', () => {
|
|||
it('resolves a pending unsubscribe when the child dies', async () => {
|
||||
const promise = subscribeViaWatcherProcess('/repo', vi.fn(), {})
|
||||
const child = currentChild()
|
||||
ackSubscribe(child)
|
||||
ackSubscribe(child, 0)
|
||||
const subscription = await promise
|
||||
const keepAlivePromise = subscribeViaWatcherProcess('/other', vi.fn(), {})
|
||||
ackSubscribe(child)
|
||||
await keepAlivePromise
|
||||
|
||||
const unsubPromise = subscription.unsubscribe()
|
||||
child.connected = false
|
||||
|
|
@ -187,6 +195,80 @@ describe('subscribeViaWatcherProcess', () => {
|
|||
)
|
||||
})
|
||||
|
||||
it('kills the idle child after the last unsubscribe and respawns on the next subscribe', async () => {
|
||||
const promise = subscribeViaWatcherProcess('/repo', vi.fn(), {})
|
||||
const first = currentChild()
|
||||
ackSubscribe(first)
|
||||
const subscription = await promise
|
||||
|
||||
await expect(subscription.unsubscribe()).resolves.toBeUndefined()
|
||||
expect(first.kill).toHaveBeenCalledTimes(1)
|
||||
|
||||
// Why: the deliberate idle kill must not count as a crash — no respawn
|
||||
// and no crash-fuse advance when the killed child's exit event lands.
|
||||
first.connected = false
|
||||
first.emit('exit', null, 'SIGTERM')
|
||||
expect(forkMock).toHaveBeenCalledTimes(1)
|
||||
|
||||
const respawnPromise = subscribeViaWatcherProcess('/repo2', vi.fn(), {})
|
||||
expect(forkMock).toHaveBeenCalledTimes(2)
|
||||
const second = currentChild()
|
||||
expect(second).not.toBe(first)
|
||||
expect(second.sent[0]).toMatchObject({ op: 'subscribe', dir: '/repo2' })
|
||||
ackSubscribe(second)
|
||||
await expect(respawnPromise).resolves.toMatchObject({ unsubscribe: expect.any(Function) })
|
||||
})
|
||||
|
||||
it('kills the idle child when the last pending subscribe fails', async () => {
|
||||
const promise = subscribeViaWatcherProcess('/gone', vi.fn(), {})
|
||||
const first = currentChild()
|
||||
first.emit('message', {
|
||||
op: 'subscribe-failed',
|
||||
id: first.sent[0].id,
|
||||
message: 'Error opening directory'
|
||||
})
|
||||
await expect(promise).rejects.toThrow('Error opening directory')
|
||||
expect(first.kill).toHaveBeenCalledTimes(1)
|
||||
|
||||
const respawnPromise = subscribeViaWatcherProcess('/repo', vi.fn(), {})
|
||||
expect(forkMock).toHaveBeenCalledTimes(2)
|
||||
ackSubscribe(currentChild())
|
||||
await expect(respawnPromise).resolves.toMatchObject({ unsubscribe: expect.any(Function) })
|
||||
})
|
||||
|
||||
it('keeps the child alive when a subscribe fails while other subscriptions remain', async () => {
|
||||
const firstPromise = subscribeViaWatcherProcess('/a', vi.fn(), {})
|
||||
const child = currentChild()
|
||||
ackSubscribe(child)
|
||||
await firstPromise
|
||||
|
||||
const failingPromise = subscribeViaWatcherProcess('/gone', vi.fn(), {})
|
||||
child.emit('message', { op: 'subscribe-failed', id: child.sent.at(-1)!.id, message: 'boom' })
|
||||
await expect(failingPromise).rejects.toThrow('boom')
|
||||
expect(child.kill).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keeps the child alive while other subscriptions remain', async () => {
|
||||
const firstPromise = subscribeViaWatcherProcess('/a', vi.fn(), {})
|
||||
const child = currentChild()
|
||||
ackSubscribe(child)
|
||||
const first = await firstPromise
|
||||
const secondPromise = subscribeViaWatcherProcess('/b', vi.fn(), {})
|
||||
ackSubscribe(child)
|
||||
const second = await secondPromise
|
||||
|
||||
const firstUnsub = first.unsubscribe()
|
||||
expect(child.kill).not.toHaveBeenCalled()
|
||||
expect(child.sent.at(-1)).toMatchObject({ op: 'unsubscribe' })
|
||||
|
||||
// The last unsubscribe kills the idle child, which also completes the
|
||||
// still-pending unsubscribe ack from the first record.
|
||||
const secondUnsub = second.unsubscribe()
|
||||
expect(child.kill).toHaveBeenCalledTimes(1)
|
||||
await expect(firstUnsub).resolves.toBeUndefined()
|
||||
await expect(secondUnsub).resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
it('uses the in-process watcher under vitest', async () => {
|
||||
vi.stubEnv('VITEST', 'true')
|
||||
const unsubscribe = vi.fn().mockResolvedValue(undefined)
|
||||
|
|
|
|||
|
|
@ -183,6 +183,9 @@ function handleChildMessage(message: WatcherToHostMessage): void {
|
|||
} else {
|
||||
record.callback(new Error(message.message), [])
|
||||
}
|
||||
// Why: a failed last subscribe empties the records map without any
|
||||
// unsubscribe ever being called — tear down the idle child here too.
|
||||
killWatcherChildIfIdle()
|
||||
return
|
||||
}
|
||||
if (message.op === 'events') {
|
||||
|
|
@ -243,6 +246,25 @@ function handleChildGone(proc: ChildProcess): void {
|
|||
}
|
||||
}
|
||||
|
||||
// Why: the last record gone leaves the child idle until app shutdown. Killing
|
||||
// it releases native handles without running watcher.node's crash-prone async
|
||||
// teardown (same rationale as disposeWatcherProcess) and reclaims the process;
|
||||
// the next subscribe forks a fresh child. `child = null` before kill so the
|
||||
// exit event neither counts as a crash nor respawns. Process death also
|
||||
// completes any still-pending unsubscribe acks.
|
||||
function killWatcherChildIfIdle(): void {
|
||||
const proc = child
|
||||
if (!proc || records.size > 0) {
|
||||
return
|
||||
}
|
||||
child = null
|
||||
for (const resolve of pendingUnsubscribes.values()) {
|
||||
resolve()
|
||||
}
|
||||
pendingUnsubscribes.clear()
|
||||
proc.kill()
|
||||
}
|
||||
|
||||
function makeSubscription(record: SubscriptionRecord): WatcherProcessSubscription {
|
||||
return {
|
||||
unsubscribe: (): Promise<void> => {
|
||||
|
|
@ -253,6 +275,10 @@ function makeSubscription(record: SubscriptionRecord): WatcherProcessSubscriptio
|
|||
if (!proc?.connected) {
|
||||
return Promise.resolve()
|
||||
}
|
||||
if (records.size === 0) {
|
||||
killWatcherChildIfIdle()
|
||||
return Promise.resolve()
|
||||
}
|
||||
return new Promise((resolve) => {
|
||||
pendingUnsubscribes.set(record.id, resolve)
|
||||
sendToChild(proc, { op: 'unsubscribe', id: record.id })
|
||||
|
|
|
|||
|
|
@ -5,7 +5,9 @@ import { join } from 'node:path'
|
|||
// assigns a random port. Paired mobile devices store ws://ip:port endpoints,
|
||||
// so a port that changes on every restart permanently orphans those pairings
|
||||
// (STA-1511). Persist the assigned fallback so the same instance re-binds the
|
||||
// same port next launch.
|
||||
// same port next launch — the transport binds a persisted fallback BEFORE the
|
||||
// preferred port, so pairings survive even when the preferred port is free
|
||||
// again.
|
||||
|
||||
const FALLBACK_PORT_FILE = 'mobile-ws-fallback-port.json'
|
||||
|
||||
|
|
|
|||
|
|
@ -454,6 +454,77 @@ describe('WebSocketTransport', () => {
|
|||
return port
|
||||
}
|
||||
|
||||
it('binds the persisted fallback port even when the preferred port is free', async () => {
|
||||
// Why: regression for the STA-1511 follow-up — devices paired while the
|
||||
// fallback port was active store ws://ip:<fallback>. A later launch that
|
||||
// finds the preferred port free must still bind the fallback, or those
|
||||
// pairings go permanently dead until the user re-pairs.
|
||||
const preferredPort = await reserveFreePort()
|
||||
const fallbackPort = await reserveFreePort()
|
||||
|
||||
const transport = new WebSocketTransport({
|
||||
host: '127.0.0.1',
|
||||
port: preferredPort,
|
||||
fallbackPort
|
||||
})
|
||||
transports.push(transport)
|
||||
await transport.start()
|
||||
expect(transport.resolvedPort).toBe(fallbackPort)
|
||||
})
|
||||
|
||||
it('binds the preferred port when the persisted fallback is taken', async () => {
|
||||
const fallbackHolder = new WebSocketTransport({ host: '127.0.0.1', port: 0 })
|
||||
transports.push(fallbackHolder)
|
||||
await fallbackHolder.start()
|
||||
const takenFallbackPort = fallbackHolder.resolvedPort
|
||||
const preferredPort = await reserveFreePort()
|
||||
|
||||
const transport = new WebSocketTransport({
|
||||
host: '127.0.0.1',
|
||||
port: preferredPort,
|
||||
fallbackPort: takenFallbackPort
|
||||
})
|
||||
transports.push(transport)
|
||||
await transport.start()
|
||||
expect(transport.resolvedPort).toBe(preferredPort)
|
||||
})
|
||||
|
||||
it('falls through to the preferred port when the fallback bind fails with a non-EADDRINUSE error', async () => {
|
||||
// Why: a persisted fallback can land in an OS-reserved range on a later
|
||||
// launch (Windows Hyper-V excluded ports → EACCES). That must degrade to
|
||||
// the preferred port instead of disabling the transport for the session.
|
||||
const preferredPort = await reserveFreePort()
|
||||
const fallbackPort = await reserveFreePort()
|
||||
const transport = new WebSocketTransport({
|
||||
host: '127.0.0.1',
|
||||
port: preferredPort,
|
||||
fallbackPort
|
||||
})
|
||||
transports.push(transport)
|
||||
const withListen = transport as unknown as { tryListen(port: number): Promise<void> }
|
||||
const realTryListen = withListen.tryListen.bind(transport)
|
||||
withListen.tryListen = (port: number) =>
|
||||
port === fallbackPort
|
||||
? Promise.reject(Object.assign(new Error('listen EACCES'), { code: 'EACCES' }))
|
||||
: realTryListen(port)
|
||||
|
||||
await transport.start()
|
||||
expect(transport.resolvedPort).toBe(preferredPort)
|
||||
})
|
||||
|
||||
it('still throws when the preferred port fails with a non-EADDRINUSE error', async () => {
|
||||
const transport = new WebSocketTransport({
|
||||
host: '127.0.0.1',
|
||||
port: await reserveFreePort()
|
||||
})
|
||||
transports.push(transport)
|
||||
const withListen = transport as unknown as { tryListen(port: number): Promise<void> }
|
||||
withListen.tryListen = () =>
|
||||
Promise.reject(Object.assign(new Error('listen EACCES'), { code: 'EACCES' }))
|
||||
|
||||
await expect(transport.start()).rejects.toThrow('listen EACCES')
|
||||
})
|
||||
|
||||
it('retries the persisted fallback port before an OS-assigned one', async () => {
|
||||
const holder = new WebSocketTransport({ host: '127.0.0.1', port: 0 })
|
||||
transports.push(holder)
|
||||
|
|
|
|||
|
|
@ -48,10 +48,11 @@ export type WebSocketTransportOptions = {
|
|||
// Why: the pairing server can also serve the browser client, so users do
|
||||
// not need a second dev/static server once the web bundle is built.
|
||||
staticRoot?: string
|
||||
// Why: when the preferred port is taken, an unstable OS-assigned port would
|
||||
// change on every restart and permanently orphan existing mobile pairings
|
||||
// (their stored ws://ip:port endpoint goes dead — STA-1511). Callers pass
|
||||
// the previously assigned fallback port so it is retried first.
|
||||
// Why: paired mobile devices store the full ws://ip:port endpoint. Once a
|
||||
// fallback port has been assigned and persisted, devices paired while it was
|
||||
// active point at it, so it must be bound FIRST on later launches — binding
|
||||
// the (now free) preferred port instead would strand those pairings
|
||||
// (STA-1511). Callers pass the previously assigned fallback port here.
|
||||
fallbackPort?: number
|
||||
}
|
||||
|
||||
|
|
@ -149,39 +150,40 @@ export class WebSocketTransport implements RpcTransport {
|
|||
return
|
||||
}
|
||||
|
||||
// Why: when the preferred port is occupied (e.g. another Orca instance is
|
||||
// already running), fall back to an OS-assigned port so mobile pairing
|
||||
// still works. The QR code reads resolvedPort after start, so it will
|
||||
// advertise the correct port regardless. A persisted fallback port is
|
||||
// retried before port 0 so paired devices keep a stable endpoint across
|
||||
// restarts (STA-1511).
|
||||
let port = this.port
|
||||
try {
|
||||
await this.tryListen(port)
|
||||
return
|
||||
} catch (error: unknown) {
|
||||
if (!isEAddressInUse(error) || port === 0) {
|
||||
throw error
|
||||
}
|
||||
}
|
||||
if (
|
||||
this.fallbackPort !== undefined &&
|
||||
this.fallbackPort !== 0 &&
|
||||
this.fallbackPort !== this.port
|
||||
) {
|
||||
// Why: a persisted fallback port is bound FIRST — devices paired while it
|
||||
// was active store ws://ip:<fallback> and would be permanently stranded if
|
||||
// a later launch grabbed the (now free) preferred port instead (STA-1511).
|
||||
// Without a persisted fallback the preferred port is tried first. On
|
||||
// EADDRINUSE each candidate falls through to the next, ending at port 0
|
||||
// (OS-assigned) so mobile pairing still works when everything is taken.
|
||||
// The QR code reads resolvedPort after start, so it always advertises the
|
||||
// port that actually bound.
|
||||
const persistedFallbackPort =
|
||||
this.fallbackPort !== undefined && this.fallbackPort !== 0 && this.fallbackPort !== this.port
|
||||
? this.fallbackPort
|
||||
: undefined
|
||||
const candidatePorts =
|
||||
persistedFallbackPort !== undefined ? [persistedFallbackPort, this.port] : [this.port]
|
||||
for (const port of candidatePorts) {
|
||||
try {
|
||||
console.warn(
|
||||
`[ws-transport] Port ${port} is in use, retrying previous fallback port ${this.fallbackPort}`
|
||||
)
|
||||
await this.tryListen(this.fallbackPort)
|
||||
await this.tryListen(port)
|
||||
return
|
||||
} catch (error: unknown) {
|
||||
if (!isEAddressInUse(error)) {
|
||||
// Why: a persisted fallback can become unbindable for reasons beyond
|
||||
// EADDRINUSE (e.g. Windows reserves dynamic-range ports for Hyper-V
|
||||
// after a reboot → EACCES). Any fallback failure must degrade to the
|
||||
// next candidate — aborting would disable the transport every launch
|
||||
// while the store still names that port. Only preferred-port failures
|
||||
// other than EADDRINUSE are fatal.
|
||||
if (port !== persistedFallbackPort && (!isEAddressInUse(error) || port === 0)) {
|
||||
throw error
|
||||
}
|
||||
console.warn(
|
||||
`[ws-transport] Failed to bind port ${port} (${error instanceof Error ? error.message : String(error)}), trying next candidate`
|
||||
)
|
||||
}
|
||||
}
|
||||
console.warn(`[ws-transport] Port ${port} is in use, falling back to OS-assigned port`)
|
||||
console.warn('[ws-transport] All configured ports failed to bind, using an OS-assigned port')
|
||||
await this.tryListen(0)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -705,8 +705,10 @@ export class OrcaRuntimeRpcServer {
|
|||
port: this.wsPort,
|
||||
staticRoot: this.webClientRoot,
|
||||
// Why: keep the fallback port stable across restarts so paired
|
||||
// devices' stored endpoints stay valid (STA-1511). wsPort 0 means
|
||||
// the caller explicitly wants a random port (E2E) — don't pin it.
|
||||
// devices' stored endpoints stay valid (STA-1511) — the transport
|
||||
// binds a persisted fallback before the preferred port. wsPort 0
|
||||
// means the caller explicitly wants a random port (E2E) — don't
|
||||
// pin it.
|
||||
...(this.wsPort !== 0 ? { fallbackPort: readWsFallbackPort(this.userDataPath) } : {})
|
||||
})
|
||||
this.wsTransport = wsTransport
|
||||
|
|
|
|||
|
|
@ -250,6 +250,43 @@ describe('attachRestoredTabConflictScan', () => {
|
|||
}
|
||||
})
|
||||
|
||||
it('caps concurrent verification reads and drains the queue without dropping tabs', async () => {
|
||||
// Why: a restored session with many dirty tabs must not fire one disk
|
||||
// read per tab at once — on SSH/remote runtimes that competes with
|
||||
// connection recovery. The cap is 3; the rest queue and all complete.
|
||||
const pendingReads: ((value: { content: string; isBinary: boolean }) => void)[] = []
|
||||
mocks.readRuntimeFileContent.mockImplementation(
|
||||
() =>
|
||||
new Promise<{ content: string; isBinary: boolean }>((resolve) => {
|
||||
pendingReads.push(resolve)
|
||||
})
|
||||
)
|
||||
const store = createEditorStore()
|
||||
for (let i = 0; i < 6; i++) {
|
||||
openRestoredDirtyTab(store, `/repo/file-${i}.ts`, 'original baseline')
|
||||
}
|
||||
|
||||
const detach = attachRestoredTabConflictScan(store)
|
||||
try {
|
||||
expect(mocks.readRuntimeFileContent).toHaveBeenCalledTimes(3)
|
||||
|
||||
pendingReads.shift()!({ content: 'original baseline', isBinary: false })
|
||||
await vi.advanceTimersByTimeAsync(10)
|
||||
expect(mocks.readRuntimeFileContent).toHaveBeenCalledTimes(4)
|
||||
|
||||
while (pendingReads.length > 0) {
|
||||
pendingReads.shift()!({ content: 'original baseline', isBinary: false })
|
||||
await vi.advanceTimersByTimeAsync(10)
|
||||
}
|
||||
expect(mocks.readRuntimeFileContent).toHaveBeenCalledTimes(6)
|
||||
for (const file of store.getState().openFiles) {
|
||||
expect(file.pendingDiskBaselineVerification).toBeUndefined()
|
||||
}
|
||||
} finally {
|
||||
detach()
|
||||
}
|
||||
})
|
||||
|
||||
it('does not mark a tab that was saved while the read was in flight', async () => {
|
||||
let resolveRead: (value: { content: string; isBinary: boolean }) => void = () => {}
|
||||
mocks.readRuntimeFileContent.mockReturnValue(
|
||||
|
|
@ -270,4 +307,55 @@ describe('attachRestoredTabConflictScan', () => {
|
|||
detach()
|
||||
}
|
||||
})
|
||||
|
||||
it('verifies a queued tab against its live baseline, not the stale queued snapshot', async () => {
|
||||
// Why: ids are paths, so a tab re-baselined (saved / reopened) while it
|
||||
// waits behind the concurrency cap must be compared against its current
|
||||
// baseline. Queuing the OpenFile snapshot would mark the live tab using the
|
||||
// old lastKnownDiskSignature.
|
||||
const pending: {
|
||||
filePath: string
|
||||
resolve: (value: { content: string; isBinary: boolean }) => void
|
||||
}[] = []
|
||||
mocks.readRuntimeFileContent.mockImplementation(
|
||||
({ filePath }: { filePath: string }) =>
|
||||
new Promise<{ content: string; isBinary: boolean }>((resolve) => {
|
||||
pending.push({ filePath, resolve })
|
||||
})
|
||||
)
|
||||
const store = createEditorStore()
|
||||
// Three reads fill the concurrency cap; the fourth waits in the queue.
|
||||
for (let i = 0; i < 4; i++) {
|
||||
openRestoredDirtyTab(store, `/repo/file-${i}.ts`, 'stale baseline')
|
||||
}
|
||||
|
||||
const detach = attachRestoredTabConflictScan(store)
|
||||
try {
|
||||
expect(mocks.readRuntimeFileContent).toHaveBeenCalledTimes(3)
|
||||
|
||||
// The queued tab is re-baselined to match current disk while it waits.
|
||||
store
|
||||
.getState()
|
||||
.setLastKnownDiskSignature('/repo/file-3.ts', getDiskBaselineSignature('current disk'))
|
||||
|
||||
// Drain the in-flight reads; each frees a slot so the queued tab dispatches.
|
||||
for (const filePath of ['/repo/file-0.ts', '/repo/file-1.ts', '/repo/file-2.ts']) {
|
||||
pending
|
||||
.find((p) => p.filePath === filePath)!
|
||||
.resolve({ content: 'stale baseline', isBinary: false })
|
||||
await vi.advanceTimersByTimeAsync(10)
|
||||
}
|
||||
|
||||
pending
|
||||
.find((p) => p.filePath === '/repo/file-3.ts')!
|
||||
.resolve({ content: 'current disk', isBinary: false })
|
||||
await vi.advanceTimersByTimeAsync(10)
|
||||
|
||||
const tab = store.getState().openFiles.find((f) => f.id === '/repo/file-3.ts')
|
||||
expect(tab?.externalMutation).toBeUndefined()
|
||||
expect(tab?.pendingDiskBaselineVerification).toBeUndefined()
|
||||
} finally {
|
||||
detach()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -29,13 +29,23 @@ type AppStoreApi = Pick<StoreApi<AppState>, 'getState' | 'subscribe'>
|
|||
const VERIFY_RETRY_MS = 2_000
|
||||
const VERIFY_SLOW_RETRY_MS = 15_000
|
||||
const VERIFY_FAST_ATTEMPTS = 30
|
||||
// Why: a session restored with many dirty tabs would otherwise fire one disk
|
||||
// read per tab simultaneously — on SSH/remote runtimes that's N concurrent
|
||||
// 15s-timeout RPCs competing with connection recovery at startup. Verifies
|
||||
// beyond the cap queue and start as slots free up; none are dropped.
|
||||
const MAX_CONCURRENT_VERIFY_READS = 3
|
||||
|
||||
export function attachRestoredTabConflictScan(store: AppStoreApi): () => void {
|
||||
// Why: dedupes in-flight verifications; the store's pending flag is the
|
||||
// durable "needs verification" signal.
|
||||
// Why: dedupes queued + in-flight verifications; the store's pending flag is
|
||||
// the durable "needs verification" signal.
|
||||
const inFlightFileIds = new Set<string>()
|
||||
const attemptsByFileId = new Map<string, number>()
|
||||
const retryTimers = new Set<ReturnType<typeof setTimeout>>()
|
||||
// Why: queue ids, not OpenFile snapshots. Ids are paths, so a snapshot can go
|
||||
// stale (close/reopen or save at the same path) while it waits for a slot;
|
||||
// the live file is re-read at dispatch instead.
|
||||
const verifyQueue: string[] = []
|
||||
let activeVerifyReads = 0
|
||||
let disposed = false
|
||||
|
||||
// Why: distinguishes "file was deleted while the app was closed" (a
|
||||
|
|
@ -144,6 +154,36 @@ export function attachRestoredTabConflictScan(store: AppStoreApi): () => void {
|
|||
}
|
||||
}
|
||||
|
||||
const pumpVerifyQueue = (): void => {
|
||||
while (!disposed && activeVerifyReads < MAX_CONCURRENT_VERIFY_READS && verifyQueue.length > 0) {
|
||||
const fileId = verifyQueue.shift()!
|
||||
// Why: re-read the live file when the slot opens. A queued id may now
|
||||
// resolve to a reopened or saved same-path tab, so re-validate it against
|
||||
// the current state before spending a disk read; skipping frees the dedupe
|
||||
// marker so a later scan can re-queue it if it needs verification again.
|
||||
const file = store.getState().openFiles.find((f) => f.id === fileId)
|
||||
if (
|
||||
!file ||
|
||||
!file.pendingDiskBaselineVerification ||
|
||||
!file.isDirty ||
|
||||
!file.lastKnownDiskSignature ||
|
||||
file.externalMutation === 'changed' ||
|
||||
!canAutoSaveOpenFile(file)
|
||||
) {
|
||||
inFlightFileIds.delete(fileId)
|
||||
continue
|
||||
}
|
||||
activeVerifyReads += 1
|
||||
const onSettled = (): void => {
|
||||
activeVerifyReads -= 1
|
||||
pumpVerifyQueue()
|
||||
}
|
||||
// Why: verify() never rejects by design, but a rejection here must still
|
||||
// release the slot or the queue stalls permanently.
|
||||
void verify(file).then(onSettled, onSettled)
|
||||
}
|
||||
}
|
||||
|
||||
const scan = (): void => {
|
||||
if (disposed) {
|
||||
return
|
||||
|
|
@ -160,8 +200,9 @@ export function attachRestoredTabConflictScan(store: AppStoreApi): () => void {
|
|||
continue
|
||||
}
|
||||
inFlightFileIds.add(file.id)
|
||||
void verify(file)
|
||||
verifyQueue.push(file.id)
|
||||
}
|
||||
pumpVerifyQueue()
|
||||
}
|
||||
|
||||
let previousOpenFiles = store.getState().openFiles
|
||||
|
|
@ -182,5 +223,6 @@ export function attachRestoredTabConflictScan(store: AppStoreApi): () => void {
|
|||
clearTimeout(timer)
|
||||
}
|
||||
retryTimers.clear()
|
||||
verifyQueue.length = 0
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue