Files
orca/src/main/ipc/remote-watcher-event-batch.test.ts
T
JinjingandOrca c5c10e1203 Fix POSIX path coalescing and file explorer watch regressions (#12004)
* fix(watch/relay): bound remote watcher fan-out and read the live relay grace

Three P1 fixes from the SSH/remote freeze audit:

- Remote watchers now debounce on the same 150/500 window as local ones
  (finding D), and every teardown path drops the trailing flush timer
  instead of letting it fire into a dead watch. The deferred send is
  wrapped so a frame disposed mid-window can't escape as a fatal
  main-process exception.
- File Explorer refreshes are scheduled and concurrency-capped rather
  than fanned out unbounded over expanded dirs (finding C). Local
  transports use a zero window, since main already coalesced the burst.
- relay.startGrace reads ptyHandler.configuredGraceTimeMs instead of the
  launch-time argv closure, so a grace raised after launch is honored.
  The branch selection moves to relay-grace-branch.ts because relay.ts
  has no exports and calls main() at import, making it untestable.
  Consequence: a host-sleep relay holding zero PTYs now exits after the
  idle cap. Pinned by test and documented in
  docs/reference/relay-grace-time-reconfiguration.md.

Also drops the duplicated 150/500/5000 constants in the runtime-RPC
batcher in favor of the shared window module.

* docs(relay): correct grace-reconfiguration line numbers after the relay.ts edit

Co-authored-by: Orca <help@stably.ai>

* refactor(file-explorer): use useMemo for paths; remove relay reference

Replace manual ref-based caching with proper React hooks for content-stable path memoization. Remove outdated relay grace-time reference documentation from code review cycle.

* rm design doc

* fix(watcher/explorer/relay): coalesce POSIX paths by byte identity; pres

- Remote watcher event coalescing now keeps NFC/NFD-distinct POSIX paths separate while still folding Windows path spellings, fixing cache invalidation when recreating directories with different Unicode compositions.
- Relay grace reconfiguration now preserves shutdown-deferred state when grace is set to zero, preventing premature shutdown when the grace timer is reconfigured mid-flight.
- File explorer refresh refactored from fixed-wave batching to concurrent task execution with `forEachWithConcurrency`, batching results every N settled reads instead of every wave, and reporting whether cancellation discarded pending work.
- Watch handler now resyncs when events arrive after disposal, ensuring refresh requests aren't lost when events race cleanup during worktree switches.

* fix(watcher/explorer): resilient commit batches on callback error

Cache writes precede callbacks, so a throwing callback cannot strand
the batch with stale marks. All callbacks complete despite errors,
with the first error thrown after.

- Fix commitBatchSize calculation for empty dirs
- Add scheduler discard-report test
- Move disposed variable before handleFsChanged
- Expand batching-strategy comments

---------

Co-authored-by: Orca <help@stably.ai>
2026-08-01 14:49:33 -07:00

307 lines
9.6 KiB
TypeScript

import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { FsChangeEvent } from '../../shared/types'
import { createRemoteWatcherEventBatch } from './remote-watcher-event-batch'
const ROOT = '/home/u/repo'
function makeBatch(deliver: (events: FsChangeEvent[]) => void, maxEvents = 5_000) {
return createRemoteWatcherEventBatch({
rootPath: ROOT,
deliver,
trailingMs: 150,
maxWaitMs: 500,
maxEvents
})
}
describe('createRemoteWatcherEventBatch', () => {
beforeEach(() => {
vi.useFakeTimers()
})
afterEach(() => {
vi.useRealTimers()
})
it('collapses many pushes inside the trailing window into one delivery', () => {
const deliver = vi.fn()
const batch = makeBatch(deliver)
for (let i = 0; i < 50; i++) {
batch.push([{ kind: 'update', absolutePath: `${ROOT}/file-${i}.ts` }])
vi.advanceTimersByTime(1)
}
expect(deliver).not.toHaveBeenCalled()
vi.advanceTimersByTime(150)
expect(deliver).toHaveBeenCalledTimes(1)
expect(deliver.mock.calls[0][0]).toHaveLength(50)
})
it('flushes at the max wait when pushes never stop', () => {
const deliver = vi.fn()
const batch = makeBatch(deliver)
for (let i = 0; i < 10; i++) {
batch.push([{ kind: 'update', absolutePath: `${ROOT}/a.ts` }])
vi.advanceTimersByTime(100)
}
expect(deliver).toHaveBeenCalled()
expect(deliver.mock.calls[0][0]).toEqual([{ kind: 'update', absolutePath: `${ROOT}/a.ts` }])
})
it('emits both events for delete then create, delete first', () => {
const deliver = vi.fn()
const batch = makeBatch(deliver)
batch.push([
{ kind: 'delete', absolutePath: `${ROOT}/a.ts` },
{ kind: 'create', absolutePath: `${ROOT}/a.ts` }
])
vi.advanceTimersByTime(150)
expect(deliver.mock.calls[0][0]).toEqual([
{ kind: 'delete', absolutePath: `${ROOT}/a.ts` },
{ kind: 'create', absolutePath: `${ROOT}/a.ts` }
])
})
it('drops both events for create then delete', () => {
const deliver = vi.fn()
const batch = makeBatch(deliver)
batch.push([
{ kind: 'create', absolutePath: `${ROOT}/tmp.ts` },
{ kind: 'delete', absolutePath: `${ROOT}/tmp.ts` }
])
vi.advanceTimersByTime(150)
expect(deliver).not.toHaveBeenCalled()
})
it('keeps a replacement delete when a directory is recreated as a file', () => {
const deliver = vi.fn()
const batch = makeBatch(deliver)
batch.push([
{ kind: 'delete', absolutePath: `${ROOT}/dist` },
{ kind: 'create', absolutePath: `${ROOT}/dist` },
{ kind: 'update', absolutePath: `${ROOT}/dist`, isDirectory: false }
])
vi.advanceTimersByTime(150)
expect(deliver.mock.calls[0][0]).toEqual([
{ kind: 'delete', absolutePath: `${ROOT}/dist` },
{ kind: 'update', absolutePath: `${ROOT}/dist`, isDirectory: false }
])
})
it('keeps the delete when a later update has unknown isDirectory', () => {
const deliver = vi.fn()
const batch = makeBatch(deliver)
batch.push([
{ kind: 'delete', absolutePath: `${ROOT}/dist` },
{ kind: 'create', absolutePath: `${ROOT}/dist` },
{ kind: 'update', absolutePath: `${ROOT}/dist` }
])
vi.advanceTimersByTime(150)
expect(deliver.mock.calls[0][0]).toEqual([
{ kind: 'delete', absolutePath: `${ROOT}/dist` },
{ kind: 'update', absolutePath: `${ROOT}/dist` }
])
})
it('emits the net delete for delete then create then delete', () => {
const deliver = vi.fn()
const batch = makeBatch(deliver)
batch.push([
{ kind: 'delete', absolutePath: `${ROOT}/dist/app.js` },
{ kind: 'create', absolutePath: `${ROOT}/dist/app.js` },
{ kind: 'delete', absolutePath: `${ROOT}/dist/app.js` }
])
vi.advanceTimersByTime(150)
expect(deliver).toHaveBeenCalledTimes(1)
expect(deliver.mock.calls[0][0]).toEqual([
{ kind: 'delete', absolutePath: `${ROOT}/dist/app.js` }
])
})
it('re-hoists the delete when the window ends on a recreate', () => {
const deliver = vi.fn()
const batch = makeBatch(deliver)
batch.push([
{ kind: 'delete', absolutePath: `${ROOT}/a.ts` },
{ kind: 'create', absolutePath: `${ROOT}/a.ts` },
{ kind: 'delete', absolutePath: `${ROOT}/a.ts` },
{ kind: 'create', absolutePath: `${ROOT}/a.ts` }
])
vi.advanceTimersByTime(150)
expect(deliver.mock.calls[0][0]).toEqual([
{ kind: 'delete', absolutePath: `${ROOT}/a.ts` },
{ kind: 'create', absolutePath: `${ROOT}/a.ts` }
])
})
it('keeps only the last update for one path', () => {
const deliver = vi.fn()
const batch = makeBatch(deliver)
batch.push([{ kind: 'update', absolutePath: `${ROOT}/a.ts`, isDirectory: false }])
batch.push([{ kind: 'update', absolutePath: `${ROOT}/a.ts`, isDirectory: false }])
vi.advanceTimersByTime(150)
expect(deliver).toHaveBeenCalledTimes(1)
expect(deliver.mock.calls[0][0]).toEqual([
{ kind: 'update', absolutePath: `${ROOT}/a.ts`, isDirectory: false }
])
})
it('discards the buffer and delivers one overflow when an overflow event arrives', () => {
const deliver = vi.fn()
const batch = makeBatch(deliver)
batch.push([{ kind: 'update', absolutePath: `${ROOT}/a.ts` }])
batch.push([{ kind: 'overflow', absolutePath: ROOT }])
batch.push([{ kind: 'update', absolutePath: `${ROOT}/b.ts` }])
vi.advanceTimersByTime(150)
expect(deliver).toHaveBeenCalledTimes(1)
expect(deliver.mock.calls[0][0]).toEqual([{ kind: 'overflow', absolutePath: ROOT }])
})
it('latches overflow once the accumulated batch exceeds maxEvents', () => {
const deliver = vi.fn()
const batch = makeBatch(deliver, 100)
for (let i = 0; i < 3; i++) {
batch.push(
Array.from(
{ length: 50 },
(_unused, index): FsChangeEvent => ({
kind: 'update',
absolutePath: `${ROOT}/wave-${i}-${index}.ts`
})
)
)
}
vi.advanceTimersByTime(150)
expect(deliver).toHaveBeenCalledTimes(1)
expect(deliver.mock.calls[0][0]).toEqual([{ kind: 'overflow', absolutePath: ROOT }])
})
it('never invents isDirectory for events that arrive without it', () => {
const deliver = vi.fn()
const batch = makeBatch(deliver)
batch.push([{ kind: 'create', absolutePath: `${ROOT}/dir` }])
vi.advanceTimersByTime(150)
const [delivered] = deliver.mock.calls[0][0] as FsChangeEvent[]
expect(delivered.isDirectory).toBeUndefined()
expect('isDirectory' in delivered).toBe(false)
})
it('passes an unknown kind through verbatim after the coalesced set', () => {
const deliver = vi.fn()
const batch = makeBatch(deliver)
batch.push([
{ kind: 'rename', absolutePath: `${ROOT}/new.ts`, oldAbsolutePath: `${ROOT}/old.ts` },
{ kind: 'update', absolutePath: `${ROOT}/a.ts` }
])
vi.advanceTimersByTime(150)
expect(deliver.mock.calls[0][0]).toEqual([
{ kind: 'update', absolutePath: `${ROOT}/a.ts` },
{ kind: 'rename', absolutePath: `${ROOT}/new.ts`, oldAbsolutePath: `${ROOT}/old.ts` }
])
})
it('delivers nothing after close', () => {
const deliver = vi.fn()
const batch = makeBatch(deliver)
batch.push([{ kind: 'update', absolutePath: `${ROOT}/a.ts` }])
batch.close()
vi.advanceTimersByTime(1_000)
expect(deliver).not.toHaveBeenCalled()
})
it('ignores a push that lands after close instead of arming a timer', () => {
const deliver = vi.fn()
const batch = makeBatch(deliver)
batch.close()
batch.push([{ kind: 'update', absolutePath: `${ROOT}/a.ts` }])
expect(vi.getTimerCount()).toBe(0)
vi.advanceTimersByTime(1_000)
expect(deliver).not.toHaveBeenCalled()
})
// Why: the host platform is irrelevant — the coalesce key must stay shape-based, so a case-sensitive
// remote pair must survive it on any host, and neither path may pick up drive/backslash rewriting.
it('keeps case-distinct remote POSIX paths separate and byte-identical', () => {
const deliver = vi.fn()
const batch = makeBatch(deliver)
batch.push([{ kind: 'update', absolutePath: '/home/u/A' }])
batch.push([{ kind: 'update', absolutePath: '/home/u/a' }])
vi.advanceTimersByTime(150)
const delivered = deliver.mock.calls[0][0] as FsChangeEvent[]
expect(delivered).toEqual([
{ kind: 'update', absolutePath: '/home/u/A' },
{ kind: 'update', absolutePath: '/home/u/a' }
])
for (const event of delivered) {
expect(event.absolutePath).not.toContain('\\')
expect(event.absolutePath).not.toMatch(/^[a-zA-Z]:/)
}
})
it('keeps NFC- and NFD-distinct remote POSIX names separate', () => {
const deliver = vi.fn()
const batch = makeBatch(deliver)
const nfcPath = `${ROOT}/caf\u00e9.txt`
const nfdPath = `${ROOT}/cafe\u0301.txt`
batch.push([{ kind: 'update', absolutePath: nfcPath }])
batch.push([{ kind: 'update', absolutePath: nfdPath }])
vi.advanceTimersByTime(150)
expect(deliver.mock.calls[0][0]).toEqual([
{ kind: 'update', absolutePath: nfcPath },
{ kind: 'update', absolutePath: nfdPath }
])
})
it('still coalesces equivalent Windows path spellings', () => {
const deliver = vi.fn()
const batch = createRemoteWatcherEventBatch({
rootPath: 'C:\\Repo',
deliver,
trailingMs: 150,
maxWaitMs: 500,
maxEvents: 5_000
})
batch.push([{ kind: 'update', absolutePath: 'C:\\Repo\\CAF\u00c9.txt' }])
batch.push([{ kind: 'update', absolutePath: 'c:/repo/cafe\u0301.txt' }])
vi.advanceTimersByTime(150)
expect(deliver.mock.calls[0][0]).toEqual([
{ kind: 'update', absolutePath: 'c:/repo/cafe\u0301.txt' }
])
})
})