mirror of
https://github.com/stablyai/orca.git
synced 2026-10-17 00:03:08 +00:00
fix(fs): degrade WSL watching gracefully on unreadable subtrees
Review fixes from live testing on real WSL: - Snapshot script: tolerate find/stat failures (`|| true`, skip-and-log in the BusyBox batch) so an unreadable subtree or mid-scan churn yields a partial snapshot instead of aborting the engine. Previously one root-owned directory (common with docker artifacts) failed both engines and marked the worktree unwatchable until app restart. - Host safety scan: treat EACCES/EPERM like ENOENT (entry invisible) in scans and native-event application, so the native engine subscribes past unreadable subtrees; unknown I/O errors still interrupt. - Canary temp dirs: embed the owner pid in the directory name and sweep dead-owner directories at host start. Hard kills from the Windows side never run the exit handler, which leaked one /tmp dir per host teardown. - Runtime bundle errors: wrap missing/corrupt manifest reads in WslWatcherCompatibilityError so they classify as permanent instead of reprobing native watching every 30s forever; drop the dead message-string branch in isPermanentWslNativeFailure. - Distro running query: request buffer output from wsl.exe so the UTF-16LE decode path actually runs and non-ASCII distro names match.
This commit is contained in:
@@ -1,9 +1,7 @@
|
||||
import { WslWatcherCompatibilityError } from './filesystem-watcher-wsl-runtime'
|
||||
|
||||
// Why: compatibility failures (unsupported distro, missing/corrupt bundle)
|
||||
// cannot heal at runtime, so reprobing native watching would spin forever.
|
||||
export function isPermanentWslNativeFailure(error: unknown): boolean {
|
||||
return (
|
||||
error instanceof WslWatcherCompatibilityError ||
|
||||
(error instanceof Error &&
|
||||
error.message.startsWith('Packaged managed WSL watcher resource is missing'))
|
||||
)
|
||||
return error instanceof WslWatcherCompatibilityError
|
||||
}
|
||||
|
||||
@@ -263,8 +263,12 @@ describe('WSL watcher recovery policy', () => {
|
||||
})
|
||||
|
||||
it('does not re-probe a packaged build with a missing managed resource', async () => {
|
||||
// Why: the runtime layer wraps missing/corrupt bundles in the
|
||||
// compatibility error, which is the only permanent-failure signal.
|
||||
ensureRuntimeMock.mockRejectedValue(
|
||||
new Error('Packaged managed WSL watcher resource is missing at C:\\Orca\\resources')
|
||||
new CompatibilityError(
|
||||
'Unreadable managed WSL watcher bundle at C:\\Orca\\resources\\wsl-watcher'
|
||||
)
|
||||
)
|
||||
const snapshot = new FakeChildProcess()
|
||||
spawnMock.mockReturnValueOnce(snapshot)
|
||||
|
||||
@@ -9,7 +9,9 @@ import {
|
||||
PROBE_BUNDLE_SENTINEL,
|
||||
parseInstallerResponse,
|
||||
parseRunningWslDistros,
|
||||
resolveWslWatcherBundlePath
|
||||
readWslWatcherBundleManifest,
|
||||
resolveWslWatcherBundlePath,
|
||||
WslWatcherCompatibilityError
|
||||
} from './filesystem-watcher-wsl-runtime'
|
||||
|
||||
const temporaryRoots: string[] = []
|
||||
@@ -77,6 +79,46 @@ describe('managed WSL watcher runtime', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('classifies missing or corrupt bundles as permanent compatibility failures', async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), 'orca-wsl-runtime-manifest-'))
|
||||
temporaryRoots.push(root)
|
||||
|
||||
await expect(readWslWatcherBundleManifest(root)).rejects.toBeInstanceOf(
|
||||
WslWatcherCompatibilityError
|
||||
)
|
||||
await writeFile(join(root, 'manifest.json'), 'not json')
|
||||
await expect(readWslWatcherBundleManifest(root)).rejects.toBeInstanceOf(
|
||||
WslWatcherCompatibilityError
|
||||
)
|
||||
await writeFile(
|
||||
join(root, 'manifest.json'),
|
||||
JSON.stringify({
|
||||
protocol: 2,
|
||||
installLayout: 1,
|
||||
nodeVersion: '24.15.0',
|
||||
bundleVersion: 'a'.repeat(20)
|
||||
})
|
||||
)
|
||||
await expect(readWslWatcherBundleManifest(root)).rejects.toThrow(
|
||||
'Invalid managed WSL watcher manifest'
|
||||
)
|
||||
await writeFile(
|
||||
join(root, 'manifest.json'),
|
||||
JSON.stringify({
|
||||
protocol: 1,
|
||||
installLayout: 1,
|
||||
nodeVersion: '24.15.0',
|
||||
bundleVersion: 'a'.repeat(20)
|
||||
})
|
||||
)
|
||||
await expect(readWslWatcherBundleManifest(root)).resolves.toEqual({
|
||||
protocol: 1,
|
||||
installLayout: 1,
|
||||
nodeVersion: '24.15.0',
|
||||
bundleVersion: 'a'.repeat(20)
|
||||
})
|
||||
})
|
||||
|
||||
it('fails closed for a missing packaged resource but resolves explicit development output', async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), 'orca-wsl-runtime-path-'))
|
||||
temporaryRoots.push(root)
|
||||
|
||||
@@ -114,14 +114,29 @@ function validateManifest(value: unknown): WslWatcherManifest {
|
||||
return manifest as WslWatcherManifest
|
||||
}
|
||||
|
||||
export async function readWslWatcherBundleManifest(
|
||||
bundlePath: string
|
||||
): Promise<WslWatcherManifest> {
|
||||
try {
|
||||
return validateManifest(JSON.parse(await readFile(join(bundlePath, 'manifest.json'), 'utf8')))
|
||||
} catch (error) {
|
||||
if (error instanceof WslWatcherCompatibilityError) {
|
||||
throw error
|
||||
}
|
||||
// Why: a missing or corrupt bundle cannot heal at runtime; classifying it
|
||||
// permanent stops the native reprobe loop from retrying every 30s forever.
|
||||
throw new WslWatcherCompatibilityError(
|
||||
`Unreadable managed WSL watcher bundle at ${bundlePath}: ${error instanceof Error ? error.message : String(error)}`
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
async function installRuntime(
|
||||
distro: string,
|
||||
signal?: AbortSignal
|
||||
): Promise<InstalledWslWatcherRuntime> {
|
||||
const bundlePath = resolveWslWatcherBundlePath()
|
||||
const manifest = validateManifest(
|
||||
JSON.parse(await readFile(join(bundlePath, 'manifest.json'), 'utf8'))
|
||||
)
|
||||
const manifest = await readWslWatcherBundleManifest(bundlePath)
|
||||
return installWslWatcherRuntime(distro, bundlePath, manifest, signal)
|
||||
}
|
||||
|
||||
@@ -224,7 +239,10 @@ export function parseRunningWslDistros(output: string | Buffer): string[] {
|
||||
|
||||
async function queryWslDistroRunning(distro: string): Promise<boolean> {
|
||||
try {
|
||||
// Why: wsl.exe emits UTF-16LE; a raw buffer lets decodeWslListOutput pick
|
||||
// the encoding so non-ASCII distro names survive instead of garbling.
|
||||
const { stdout } = await execFileAsync('wsl.exe', ['--list', '--running', '--quiet'], {
|
||||
encoding: 'buffer',
|
||||
timeout: 5_000,
|
||||
maxBuffer: 64 * 1024
|
||||
})
|
||||
|
||||
@@ -109,25 +109,31 @@ done
|
||||
)
|
||||
|
||||
it.skipIf(process.platform === 'win32')(
|
||||
'surfaces real portable stat failures instead of treating them as churn',
|
||||
'degrades unreadable entries to a partial frame instead of aborting the poll loop',
|
||||
async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), 'orca-wsl-portable-error-'))
|
||||
temporaryRoots.push(root)
|
||||
const denied = join(root, 'denied.md')
|
||||
const readable = join(root, 'readable.md')
|
||||
await writeFile(denied, 'still exists')
|
||||
await writeFile(readable, 'fine')
|
||||
const result = await runSnapshotScript(
|
||||
root,
|
||||
`#!/bin/sh
|
||||
shift 3
|
||||
if test "$#" -gt 1; then exit 1; fi
|
||||
if test "$1" = ${JSON.stringify(denied)}; then echo 'permission denied' >&2; exit 1; fi
|
||||
printf 'directory\\t1\\n'
|
||||
printf 'regular file\\t1\\n'
|
||||
`
|
||||
)
|
||||
|
||||
expect(result.code).toEqual(expect.any(Number))
|
||||
expect(result.code).not.toBe(0)
|
||||
// Why: one unreadable subtree must not orphan the whole worktree; the
|
||||
// frame still terminates so the engine keeps polling the readable rest.
|
||||
expect(result.code).toBe(0)
|
||||
expect(result.stderr).toContain('permission denied')
|
||||
const frame = result.stdout.toString('utf8')
|
||||
expect(frame).toContain(`regular file\t1\t${readable}\0`)
|
||||
expect(frame.endsWith('\0\0') || frame === '\0').toBe(true)
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
@@ -38,7 +38,8 @@ function buildPruneExpression(ignoreDirs: readonly string[]): string {
|
||||
}
|
||||
|
||||
// Why: files can vanish after find batches them. Retry a failed batch one path
|
||||
// at a time so churn is skipped without hiding errors for paths that still exist.
|
||||
// at a time so churn is skipped; unreadable survivors are logged and skipped
|
||||
// too, because a partial snapshot beats aborting the whole watcher.
|
||||
const BUSYBOX_STAT_BATCH_SCRIPT =
|
||||
'tmp="${TMPDIR:-/tmp}/orca-wsl-snapshot-$$"; error="$tmp.error"; ' +
|
||||
'trap \'rm -f "$tmp" "$error"\' EXIT; ' +
|
||||
@@ -48,7 +49,7 @@ const BUSYBOX_STAT_BATCH_SCRIPT =
|
||||
'else for path do if metadata=$(stat -c "%F\t%y" -- "$path" 2>"$error"); then ' +
|
||||
'printf "%s\\t%s\\0" "$metadata" "$path"; ' +
|
||||
'elif test ! -e "$path" && test ! -L "$path"; then :; ' +
|
||||
'else cat "$error" >&2; exit 75; fi; done; fi'
|
||||
'else cat "$error" >&2; fi; done; fi'
|
||||
|
||||
function quoteShellArgument(value: string): string {
|
||||
return `'${value.replaceAll("'", `'"'"'`)}'`
|
||||
@@ -79,8 +80,10 @@ export function buildSnapshotScript(
|
||||
' if [ -d "$root" ]; then',
|
||||
// Why: GNU find is the zero-fork fast path. BusyBox batches paths through
|
||||
// one stat process per ARG_MAX group while retaining NUL-safe filenames.
|
||||
` if [ "$snapshot_find" = gnu ]; then find "$root" -mindepth 1 ${prune} -printf '%y\\t%T@\\t%p\\0';`,
|
||||
` else find "$root" -mindepth 1 ${prune} -exec sh -c ${quoteShellArgument(BUSYBOX_STAT_BATCH_SCRIPT)} sh {} +; fi`,
|
||||
// Why: `|| true` — an unreadable subtree or mid-scan churn must degrade to
|
||||
// a partial snapshot, not abort the engine and orphan the whole worktree.
|
||||
` if [ "$snapshot_find" = gnu ]; then find "$root" -mindepth 1 ${prune} -printf '%y\\t%T@\\t%p\\0' || true;`,
|
||||
` else find "$root" -mindepth 1 ${prune} -exec sh -c ${quoteShellArgument(BUSYBOX_STAT_BATCH_SCRIPT)} sh {} + || true; fi`,
|
||||
' fi',
|
||||
" printf '\\0'",
|
||||
repeat,
|
||||
|
||||
@@ -1,12 +1,15 @@
|
||||
import { spawn } from 'node:child_process'
|
||||
import { existsSync } from 'node:fs'
|
||||
import { existsSync, mkdirSync } from 'node:fs'
|
||||
import { mkdtemp, readFile, rm } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join, resolve } from 'node:path'
|
||||
import { build } from 'esbuild'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { createWatcherBindingWatchdog } from './wsl-watcher-host-binding-watchdog'
|
||||
import { startWslWatcherCanary } from './wsl-watcher-host-canary'
|
||||
import {
|
||||
startWslWatcherCanary,
|
||||
sweepStaleWslWatcherCanaryDirectories
|
||||
} from './wsl-watcher-host-canary'
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
@@ -130,6 +133,28 @@ describe('WSL watcher host canary', () => {
|
||||
expect(vi.getTimerCount()).toBe(0)
|
||||
})
|
||||
|
||||
it('sweeps only dead-owner canary directories at host start', async () => {
|
||||
const base = await mkdtemp(join(tmpdir(), 'orca-canary-sweep-test-'))
|
||||
try {
|
||||
const stale = join(base, 'orca-wsl-watcher-4242-abc123')
|
||||
const live = join(base, 'orca-wsl-watcher-5151-def456')
|
||||
const own = join(base, `orca-wsl-watcher-${process.pid}-ghi789`)
|
||||
const unrelated = join(base, 'orca-wsl-watcher-legacyname')
|
||||
for (const dir of [stale, live, own, unrelated]) {
|
||||
mkdirSync(dir)
|
||||
}
|
||||
|
||||
sweepStaleWslWatcherCanaryDirectories(base, (pid) => pid === 5151)
|
||||
|
||||
expect(existsSync(stale)).toBe(false)
|
||||
expect(existsSync(live)).toBe(true)
|
||||
expect(existsSync(own)).toBe(true)
|
||||
expect(existsSync(unrelated)).toBe(true)
|
||||
} finally {
|
||||
await rm(base, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
it('unsubscribes and cleans resources on an ordinary close', async () => {
|
||||
vi.useFakeTimers()
|
||||
let canaryDir = ''
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { existsSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import type { WatcherBindingWatchdog } from './wsl-watcher-host-binding-watchdog'
|
||||
@@ -13,12 +13,41 @@ export type WslWatcherCanary = { close(): Promise<void> }
|
||||
const CANARY_INTERVAL_MS = 10_000
|
||||
const CANARY_EVENT_TIMEOUT_MS = 5_000
|
||||
const CANARY_MAX_MISSES = 2
|
||||
const CANARY_DIR_PATTERN = /^orca-wsl-watcher-(\d+)-/
|
||||
|
||||
// Why: hard kills from the Windows side (wsl.exe teardown) never run the
|
||||
// 'exit' handler, so each host start reclaims directories whose owner died.
|
||||
export function sweepStaleWslWatcherCanaryDirectories(
|
||||
baseDir: string = tmpdir(),
|
||||
isProcessAlive: (pid: number) => boolean = (pid) => existsSync(`/proc/${pid}`)
|
||||
): void {
|
||||
let entries: string[]
|
||||
try {
|
||||
entries = readdirSync(baseDir)
|
||||
} catch {
|
||||
return
|
||||
}
|
||||
for (const entry of entries) {
|
||||
const owner = Number(CANARY_DIR_PATTERN.exec(entry)?.[1])
|
||||
if (!Number.isSafeInteger(owner) || owner === process.pid || isProcessAlive(owner)) {
|
||||
continue
|
||||
}
|
||||
try {
|
||||
rmSync(join(baseDir, entry), { recursive: true, force: true })
|
||||
} catch {
|
||||
// Another sweeper or the filesystem may win the race; try again next start.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export async function startWslWatcherCanary(
|
||||
binding: CanaryWatcherBinding,
|
||||
watchdog: WatcherBindingWatchdog
|
||||
): Promise<WslWatcherCanary> {
|
||||
const dir = mkdtempSync(join(tmpdir(), 'orca-wsl-watcher-'))
|
||||
if (process.platform === 'linux') {
|
||||
sweepStaleWslWatcherCanaryDirectories()
|
||||
}
|
||||
const dir = mkdtempSync(join(tmpdir(), `orca-wsl-watcher-${process.pid}-`))
|
||||
let lastEventAt = 0
|
||||
const callback = (error: Error | null): void => {
|
||||
if (!error) {
|
||||
|
||||
@@ -111,6 +111,14 @@ export function fileSystemErrorCode(error: unknown): string | undefined {
|
||||
return (error as NodeJS.ErrnoException | undefined)?.code
|
||||
}
|
||||
|
||||
// Why: unreadable entries are invisible to every engine (native watches and
|
||||
// snapshot scans alike), so treating them as absent keeps a worktree with one
|
||||
// root-owned subtree watchable instead of failing the whole subscription.
|
||||
export function isInvisibleFileSystemError(error: unknown): boolean {
|
||||
const code = fileSystemErrorCode(error)
|
||||
return code === 'ENOENT' || code === 'EACCES' || code === 'EPERM'
|
||||
}
|
||||
|
||||
export function createSafetySnapshotEntry(stats: Stats): SafetySnapshotEntry {
|
||||
const directory = stats.isDirectory()
|
||||
return {
|
||||
@@ -260,7 +268,7 @@ async function scanWithLimits(
|
||||
return statResult
|
||||
}
|
||||
if (statResult.kind === 'error') {
|
||||
if (fileSystemErrorCode(statResult.error) === 'ENOENT') {
|
||||
if (isInvisibleFileSystemError(statResult.error)) {
|
||||
continue
|
||||
}
|
||||
return { kind: 'io-error' }
|
||||
@@ -283,7 +291,7 @@ async function scanWithLimits(
|
||||
return readResult
|
||||
}
|
||||
if (readResult.kind === 'error') {
|
||||
if (fileSystemErrorCode(readResult.error) !== 'ENOENT') {
|
||||
if (!isInvisibleFileSystemError(readResult.error)) {
|
||||
return { kind: 'io-error' }
|
||||
}
|
||||
continue
|
||||
|
||||
@@ -110,6 +110,21 @@ describe('WSL watcher host safety', () => {
|
||||
).resolves.toBe('topology')
|
||||
expect(snapshot.get(directory)).toEqual(baseline)
|
||||
|
||||
const failing = join(root, 'failing')
|
||||
await expect(
|
||||
applyNativeEventsToSafetySnapshot(root, snapshot, [{ type: 'update', path: failing }], {
|
||||
fileSystem: {
|
||||
lstat: async () => {
|
||||
throw Object.assign(new Error('io failure'), { code: 'EIO' })
|
||||
},
|
||||
readdir: async () => []
|
||||
}
|
||||
})
|
||||
).resolves.toBe('topology')
|
||||
expect(snapshot.get(directory)).toEqual(baseline)
|
||||
|
||||
// Why: unreadable paths are invisible to safety scans, so events about
|
||||
// them must not interrupt the subscription the way unknown I/O does.
|
||||
const denied = join(root, 'denied')
|
||||
await expect(
|
||||
applyNativeEventsToSafetySnapshot(root, snapshot, [{ type: 'update', path: denied }], {
|
||||
@@ -120,10 +135,38 @@ describe('WSL watcher host safety', () => {
|
||||
readdir: async () => []
|
||||
}
|
||||
})
|
||||
).resolves.toBe('topology')
|
||||
).resolves.toBe('applied')
|
||||
expect(snapshot.has(denied)).toBe(false)
|
||||
expect(snapshot.get(directory)).toEqual(baseline)
|
||||
})
|
||||
|
||||
it('scans past unreadable subtrees instead of failing the subscription', async () => {
|
||||
const root = makeTemporaryRoot()
|
||||
const locked = join(root, 'locked')
|
||||
const blocked = join(root, 'blocked.md')
|
||||
const readable = join(root, 'readable.md')
|
||||
mkdirSync(locked)
|
||||
writeFileSync(join(locked, 'secret.md'), 'hidden')
|
||||
writeFileSync(blocked, 'lstat denied')
|
||||
writeFileSync(readable, 'fine')
|
||||
const { lstat, readdir } = await import('node:fs/promises')
|
||||
const denied = (): Promise<never> =>
|
||||
Promise.reject(Object.assign(new Error('denied'), { code: 'EACCES' }))
|
||||
const result = await scanWatcherSafetySnapshot(root, new Set(), {
|
||||
fileSystem: {
|
||||
lstat: (path) => (path === blocked ? denied() : lstat(path)),
|
||||
readdir: (path) => (path === locked ? denied() : readdir(path, { withFileTypes: true }))
|
||||
}
|
||||
})
|
||||
|
||||
expect(result.kind).toBe('complete')
|
||||
const snapshot = (result as Extract<typeof result, { kind: 'complete' }>).snapshot
|
||||
expect(snapshot.has(readable)).toBe(true)
|
||||
expect(snapshot.get(locked)?.directory).toBe(true)
|
||||
expect(snapshot.has(join(locked, 'secret.md'))).toBe(false)
|
||||
expect(snapshot.has(blocked)).toBe(false)
|
||||
})
|
||||
|
||||
it('bounds asynchronous native-event inspection without committing partial state', async () => {
|
||||
const snapshot: SafetySnapshot = new Map([
|
||||
['/repo', { directory: true, signature: 'd:1' }],
|
||||
|
||||
@@ -2,7 +2,7 @@ import {
|
||||
boundedFileSystemOperation,
|
||||
createSafetySnapshotEntry,
|
||||
defaultSafetyFileSystem,
|
||||
fileSystemErrorCode,
|
||||
isInvisibleFileSystemError,
|
||||
type SafetyScanOptions,
|
||||
type SafetySnapshot,
|
||||
type SafetySnapshotEntry
|
||||
@@ -87,9 +87,9 @@ export async function applyNativeEventsToSafetySnapshot(
|
||||
return 'topology'
|
||||
}
|
||||
if (statResult.kind === 'error') {
|
||||
if (fileSystemErrorCode(statResult.error) !== 'ENOENT') {
|
||||
// Why: keep the known-good baseline when permissions or I/O failures
|
||||
// leave the path's recursive-watch topology uncertain.
|
||||
// Why: vanished and unreadable paths are equally invisible to safety
|
||||
// scans; only unknown I/O failures leave the topology uncertain.
|
||||
if (!isInvisibleFileSystemError(statResult.error)) {
|
||||
return 'topology'
|
||||
}
|
||||
if (previous?.directory) {
|
||||
|
||||
Reference in New Issue
Block a user