From 3c87873dca56f53ce9636faf0cadac0dc12fe1ce Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 12 Jul 2026 18:05:11 -0700 Subject: [PATCH 1/4] Fix Command Code smart-sort bump on new prompt while working (STA-350) (#5286) Co-authored-by: Orca --- src/main/agent-hooks/server.test.ts | 24 +++++ src/main/agent-hooks/server.ts | 18 +++- src/main/index.ts | 2 + .../sidebar/smart-attention.test.ts | 15 +++ .../src/components/sidebar/smart-attention.ts | 13 ++- src/renderer/src/hooks/useIpcEvents.ts | 5 +- .../src/runtime/web-session-tabs-sync.test.ts | 75 +++++++++++++++ .../src/runtime/web-session-tabs-sync.ts | 17 +++- .../src/store/slices/agent-status.test.ts | 95 +++++++++++++++++++ src/renderer/src/store/slices/agent-status.ts | 62 ++++++++++-- src/shared/agent-status-types.ts | 4 + src/shared/command-code-turn-boundary.test.ts | 71 ++++++++++++++ src/shared/command-code-turn-boundary.ts | 56 +++++++++++ 13 files changed, 443 insertions(+), 14 deletions(-) create mode 100644 src/shared/command-code-turn-boundary.test.ts create mode 100644 src/shared/command-code-turn-boundary.ts diff --git a/src/main/agent-hooks/server.test.ts b/src/main/agent-hooks/server.test.ts index 9e9676c2243..ba843ad9856 100644 --- a/src/main/agent-hooks/server.test.ts +++ b/src/main/agent-hooks/server.test.ts @@ -3330,6 +3330,30 @@ describe('AgentHookServer prompt-sent telemetry', () => { expect(trackMock).toHaveBeenCalledTimes(2) }) + it('includes Command Code prompt interaction keys in the IPC snapshot', () => { + const server = new AgentHookServer() + + server.ingestRemote( + { + paneKey: PANE, + tabId: 'tab-1', + worktreeId: 'wt-1', + hasExplicitPrompt: true, + promptInteractionKey: 'command-code-transcript-user-1', + payload: { state: 'working', prompt: 'rerun', agentType: 'command-code' } + }, + 'conn-1' + ) + + expect(server.getStatusSnapshot()[0]).toMatchObject({ + paneKey: PANE, + promptInteractionKey: 'command-code-transcript-user-1', + state: 'working', + prompt: 'rerun', + agentType: 'command-code' + }) + }) + it('dedupes Command Code direct prompt hooks followed by transcript-backed stop hooks', () => { const server = new AgentHookServer() diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index f859277303d..32954344084 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -59,6 +59,7 @@ import { import { parseLegacyNumericPaneKey, parsePaneKey } from '../../shared/stable-pane-id' import type { LegacyPaneKeyAliasEntry } from '../../shared/types' import { normalizeAgentProviderSession } from '../../shared/agent-session-resume' +import { isCommandCodeNewTurnWhileWorking } from '../../shared/command-code-turn-boundary' export type { AgentHookSource } @@ -251,6 +252,7 @@ function toAgentStatusIpcPayload(entry: EnrichedAgentHookEventPayload): AgentSta receivedAt: entry.receivedAt, stateStartedAt: entry.stateStartedAt, ...(entry.providerSession ? { providerSession: entry.providerSession } : {}), + ...(entry.promptInteractionKey ? { promptInteractionKey: entry.promptInteractionKey } : {}), ...entry.payload } } @@ -653,8 +655,22 @@ export class AgentHookServer { const previous = this.state.lastStatusByPaneKey.get(payload.paneKey) as | EnrichedAgentHookEventPayload | undefined + const commandCodeNewTurn = + previous !== undefined && + isCommandCodeNewTurnWhileWorking({ + agentType: payload.payload.agentType, + previousState: previous.payload.state, + incomingState: payload.payload.state, + previousPrompt: previous.payload.prompt, + incomingPrompt: payload.payload.prompt, + hasExplicitPrompt: payload.hasExplicitPrompt, + previousPromptInteractionKey: previous.promptInteractionKey, + incomingPromptInteractionKey: payload.promptInteractionKey + }) const stateStartedAt = - previous && previous.payload.state === payload.payload.state ? previous.stateStartedAt : now + previous && previous.payload.state === payload.payload.state && !commandCodeNewTurn + ? previous.stateStartedAt + : now return { ...payload, receivedAt: now, diff --git a/src/main/index.ts b/src/main/index.ts index 85b62682f5d..16d3c870c2f 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -1022,6 +1022,7 @@ function openMainWindow(): BrowserWindow { stateStartedAt, launchToken, providerSession, + promptInteractionKey, isReplay }) => { if (mainWindow?.isDestroyed()) { @@ -1041,6 +1042,7 @@ function openMainWindow(): BrowserWindow { receivedAt, stateStartedAt, ...(providerSession ? { providerSession } : {}), + ...(promptInteractionKey ? { promptInteractionKey } : {}), ...(orchestration ? { orchestration } : {}) }) recordAgentStateCrashBreadcrumb(payload.agentType ?? 'unknown', payload.state) diff --git a/src/renderer/src/components/sidebar/smart-attention.test.ts b/src/renderer/src/components/sidebar/smart-attention.test.ts index 96b54f9a5c8..be04dd3e90d 100644 --- a/src/renderer/src/components/sidebar/smart-attention.test.ts +++ b/src/renderer/src/components/sidebar/smart-attention.test.ts @@ -188,6 +188,21 @@ describe('resolveAttention', () => { }) }) + it('uses a reset stateStartedAt for Command Code new prompts while still working', () => { + const entry = makeEntry({ + paneKey: 't:1', + state: 'working', + agentType: 'command-code', + stateStartedAt: NOW - 2_000, + updatedAt: NOW - 500, + stateHistory: [makeHistory('done', NOW - 30 * 60_000)] + }) + expect(resolveAttention([hookPane(entry)], NOW)).toEqual({ + cls: 3, + attentionTimestamp: NOW - 2_000 + }) + }) + it('falls back to current stateStartedAt when working has no prior attention history', () => { const entry = makeEntry({ paneKey: 't:1', diff --git a/src/renderer/src/components/sidebar/smart-attention.ts b/src/renderer/src/components/sidebar/smart-attention.ts index ae0d7542da3..8b47216cfa2 100644 --- a/src/renderer/src/components/sidebar/smart-attention.ts +++ b/src/renderer/src/components/sidebar/smart-attention.ts @@ -152,7 +152,18 @@ export function resolveAttention(panes: PaneInput[], now: number): WorktreeAtten // been working for an hour. Falls back to the current stateStartedAt // when stateHistory is empty (e.g. fresh after restart). const prior = mostRecentAttentionInHistory(entry.stateHistory) - ts = prior ?? entry.stateStartedAt + if (prior === null) { + ts = entry.stateStartedAt + } else if (entry.agentType === 'command-code') { + // Why: Command Code has no UserPromptSubmit hook, so a new prompt while + // still `working` only advances stateStartedAt (no new history row). It + // must beat the stale prior-attention timestamp. Other agents keep the + // prior-attention ordering — their real state transitions already mark + // the turn boundary, so scoping avoids reordering them. + ts = Math.max(prior, entry.stateStartedAt) + } else { + ts = prior + } } } else { // Title-heuristic fallback (no fresh hook entry for this pane). Hook diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index cd72db3a44b..9db4c0d3bf4 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -3029,6 +3029,9 @@ export function useIpcEvents(): void { const statusPayload = data.orchestration ? { ...resolvedPayload, orchestration: data.orchestration } : resolvedPayload + const statusPayloadWithTurnBoundary = data.promptInteractionKey + ? { ...statusPayload, promptInteractionKey: data.promptInteractionKey } + : statusPayload const existingStatus = store.agentStatusByPaneKey[data.paneKey] if (existingStatus && data.receivedAt < existingStatus.updatedAt) { // Why: the store rejects out-of-order status rows; keep notification and @@ -3075,7 +3078,7 @@ export function useIpcEvents(): void { const statusWorktreeId = data.worktreeId ?? owningWorktreeId store.setAgentStatus( data.paneKey, - statusPayload, + statusPayloadWithTurnBoundary, terminalTitle, { updatedAt: data.receivedAt, diff --git a/src/renderer/src/runtime/web-session-tabs-sync.test.ts b/src/renderer/src/runtime/web-session-tabs-sync.test.ts index 7090ac68bc1..cab8071cfa9 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync.test.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync.test.ts @@ -1697,6 +1697,81 @@ describe('applyWebSessionTabsSnapshot', () => { }) }) + it('bumps sort epoch for mirrored Command Code same-state turn starts', () => { + const hostPaneKey = makePaneKey('host-tab-1', LEAF_ID) + const initialPatch = applyWebSessionTabsSnapshot( + makeState(), + makeSnapshot([ + { + type: 'terminal', + id: HOST_SURFACE_ID, + title: 'Command Code', + parentTabId: 'host-tab-1', + leafId: LEAF_ID, + isActive: true, + status: 'ready', + terminal: 'terminal-1', + agentStatus: { + state: 'working', + prompt: 'same prompt', + updatedAt: NOW - 1_000, + stateStartedAt: NOW - 1_000, + agentType: 'command-code', + paneKey: hostPaneKey, + terminalTitle: 'Command Code', + stateHistory: [], + promptInteractionKey: 'command-code-transcript-a' + } + } + ]), + ENV, + NOW + ) as Partial + const initialState = { ...makeState(), ...initialPatch } + const mirroredId = initialPatch.tabsByWorktree?.[WT]?.[0]?.id + const mirroredPaneKey = makePaneKey(mirroredId!, LEAF_ID) + + const patch = applyWebSessionTabsSnapshot( + initialState, + makeSnapshot( + [ + { + type: 'terminal', + id: HOST_SURFACE_ID, + title: 'Command Code', + parentTabId: 'host-tab-1', + leafId: LEAF_ID, + isActive: true, + status: 'ready', + terminal: 'terminal-1', + agentStatus: { + state: 'working', + prompt: 'same prompt', + updatedAt: NOW, + stateStartedAt: NOW, + agentType: 'command-code', + paneKey: hostPaneKey, + terminalTitle: 'Command Code', + stateHistory: [], + promptInteractionKey: 'command-code-transcript-b' + } + } + ], + { snapshotVersion: 2 } + ), + ENV, + NOW + ) as Partial + + expect(patch.agentStatusByPaneKey?.[mirroredPaneKey]).toMatchObject({ + prompt: 'same prompt', + stateStartedAt: NOW, + promptInteractionKey: 'command-code-transcript-b' + }) + expect(patch.agentStatusEpoch).toBe((initialState.agentStatusEpoch ?? 0) + 1) + expect(patch.sortEpoch).toBe((initialState.sortEpoch ?? 0) + 1) + }) + it('hydrates multiple initial host snapshots in one merged patch', () => { const secondWorktree = 'repo::/other-worktree' const patch = applyWebSessionTabsSnapshots( diff --git a/src/renderer/src/runtime/web-session-tabs-sync.ts b/src/renderer/src/runtime/web-session-tabs-sync.ts index e28072d5357..df81b919276 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync.ts @@ -716,7 +716,8 @@ function buildMirroredAgentStatusPatch( sortRelevantChange || !existing || existing.state !== entry.state || - !isAgentStatusFresh(existing, now) + !isAgentStatusFresh(existing, now) || + isMirroredCommandCodeTurnBump(existing, entry) } if (!changed) { @@ -1297,6 +1298,7 @@ function agentStatusEntryEqual(a: AgentStatusEntry | undefined, b: AgentStatusEn a.interactivePrompt === b.interactivePrompt && a.lastAssistantMessage === b.lastAssistantMessage && a.interrupted === b.interrupted && + a.promptInteractionKey === b.promptInteractionKey && sameAgentStateHistory(a.stateHistory, b.stateHistory) ) } @@ -1305,6 +1307,19 @@ function isAgentStatusFresh(entry: Pick, now: num return now - entry.updatedAt <= AGENT_STATUS_STALE_AFTER_MS } +function isMirroredCommandCodeTurnBump( + existing: AgentStatusEntry | undefined, + entry: AgentStatusEntry +): boolean { + return ( + existing?.agentType === 'command-code' && + entry.agentType === 'command-code' && + existing.state === 'working' && + entry.state === 'working' && + entry.stateStartedAt > existing.stateStartedAt + ) +} + function sameStringRecord( a: Readonly> | undefined, b: Readonly> | undefined diff --git a/src/renderer/src/store/slices/agent-status.test.ts b/src/renderer/src/store/slices/agent-status.test.ts index cf3b7cd9b12..1aa12701207 100644 --- a/src/renderer/src/store/slices/agent-status.test.ts +++ b/src/renderer/src/store/slices/agent-status.test.ts @@ -664,6 +664,101 @@ describe('agent status tool + assistant fields', () => { expect(store.getState().agentStatusEpoch).toBe(firstEpoch + 1) expect(store.getState().sortEpoch).toBe(firstSortEpoch + 1) }) + + it('bumps sort epoch when Command Code starts a new prompt while still working', () => { + vi.useFakeTimers() + const store = createTestStore() + store + .getState() + .setAgentStatus( + 'tab-1:1', + { state: 'working', prompt: 'first task', agentType: 'command-code' }, + 'command-code', + { updatedAt: 1_000, stateStartedAt: 1_000 } + ) + const firstSortEpoch = store.getState().sortEpoch + + store + .getState() + .setAgentStatus( + 'tab-1:1', + { state: 'working', prompt: 'second task', agentType: 'command-code' }, + 'command-code', + { updatedAt: 2_000, stateStartedAt: 2_000 } + ) + + const entry = store.getState().agentStatusByPaneKey['tab-1:1'] + expect(entry.prompt).toBe('second task') + expect(entry.stateStartedAt).toBe(2_000) + expect(store.getState().sortEpoch).toBe(firstSortEpoch + 1) + }) + + it('bumps sort epoch when Command Code reruns the same prompt with a new turn key', () => { + vi.useFakeTimers() + const store = createTestStore() + store.getState().setAgentStatus( + 'tab-1:1', + { + state: 'working', + prompt: 'same task', + agentType: 'command-code', + promptInteractionKey: 'command-code-transcript-a' + }, + 'command-code', + { updatedAt: 1_000, stateStartedAt: 1_000 } + ) + const firstSortEpoch = store.getState().sortEpoch + + store.getState().setAgentStatus( + 'tab-1:1', + { + state: 'working', + prompt: 'same task', + agentType: 'command-code', + promptInteractionKey: 'command-code-transcript-b' + }, + 'command-code', + { updatedAt: 2_000, stateStartedAt: 2_000 } + ) + + const entry = store.getState().agentStatusByPaneKey['tab-1:1'] + expect(entry.prompt).toBe('same task') + expect(entry.promptInteractionKey).toBe('command-code-transcript-b') + expect(entry.stateStartedAt).toBe(2_000) + expect(store.getState().sortEpoch).toBe(firstSortEpoch + 1) + }) + + it('bumps sort epoch when main advances Command Code stateStartedAt without a renderer-visible key change', () => { + vi.useFakeTimers() + const store = createTestStore() + // First turn carries no interaction key (e.g. transcript read failed), so + // the renderer stores no promptInteractionKey to compare against. + store + .getState() + .setAgentStatus( + 'tab-1:1', + { state: 'working', prompt: 'same task', agentType: 'command-code' }, + 'command-code', + { updatedAt: 1_000, stateStartedAt: 1_000 } + ) + const firstSortEpoch = store.getState().sortEpoch + + // Main detected a new turn via interaction-key change and reset stateStartedAt, + // but the renderer can't see the key change (no key, identical prompt text). + // The authoritative stateStartedAt advance must still re-sort. + store + .getState() + .setAgentStatus( + 'tab-1:1', + { state: 'working', prompt: 'same task', agentType: 'command-code' }, + 'command-code', + { updatedAt: 2_000, stateStartedAt: 2_000 } + ) + + const entry = store.getState().agentStatusByPaneKey['tab-1:1'] + expect(entry.stateStartedAt).toBe(2_000) + expect(store.getState().sortEpoch).toBe(firstSortEpoch + 1) + }) }) describe('agent status PR refresh handoff', () => { diff --git a/src/renderer/src/store/slices/agent-status.ts b/src/renderer/src/store/slices/agent-status.ts index 99ae087f5b9..d135a5853ac 100644 --- a/src/renderer/src/store/slices/agent-status.ts +++ b/src/renderer/src/store/slices/agent-status.ts @@ -23,6 +23,7 @@ import { resolveAgentStatusIdentity, shouldSuppressInheritedTerminalStatus } from '../../../../shared/agent-status-identity' +import { isCommandCodeNewTurnWhileWorking } from '../../../../shared/command-code-turn-boundary' import type { TerminalTab } from '../../../../shared/types' import { isExplicitAgentStatusFresh } from '@/lib/agent-status' import { @@ -133,7 +134,10 @@ export type AgentStatusSlice = { /** Update or insert an agent status entry from a status payload. */ setAgentStatus: ( paneKey: string, - payload: ParsedAgentStatusPayload & { orchestration?: AgentStatusOrchestrationContext }, + payload: ParsedAgentStatusPayload & { + orchestration?: AgentStatusOrchestrationContext + promptInteractionKey?: string + }, terminalTitle?: string, timing?: { updatedAt?: number; stateStartedAt?: number }, routing?: { tabId?: string; worktreeId?: string; terminalHandle?: string }, @@ -1229,14 +1233,6 @@ export const createAgentStatusSlice: StateCreator { + it('returns true when Command Code gets a new transcript prompt while still working', () => { + expect( + isCommandCodeNewTurnWhileWorking({ + agentType: 'command-code', + previousState: 'working', + incomingState: 'working', + previousPrompt: 'first task', + incomingPrompt: 'second task', + hasExplicitPrompt: true + }) + ).toBe(true) + }) + + it('returns true when the prompt interaction key changes', () => { + expect( + isCommandCodeNewTurnWhileWorking({ + agentType: 'command-code', + previousState: 'working', + incomingState: 'working', + previousPrompt: 'same text', + incomingPrompt: 'same text', + hasExplicitPrompt: true, + previousPromptInteractionKey: 'command-code-transcript-a', + incomingPromptInteractionKey: 'command-code-transcript-b' + }) + ).toBe(true) + }) + + it('returns false for same-turn tool pings that keep the same prompt', () => { + expect( + isCommandCodeNewTurnWhileWorking({ + agentType: 'command-code', + previousState: 'working', + incomingState: 'working', + previousPrompt: 'run pwd', + incomingPrompt: 'run pwd', + hasExplicitPrompt: true + }) + ).toBe(false) + }) + + it('returns false when main-process hooks explicitly deny prompt evidence', () => { + expect( + isCommandCodeNewTurnWhileWorking({ + agentType: 'command-code', + previousState: 'working', + incomingState: 'working', + previousPrompt: 'first', + incomingPrompt: 'second', + hasExplicitPrompt: false + }) + ).toBe(false) + }) + + it('returns false for non-Command Code agents', () => { + expect( + isCommandCodeNewTurnWhileWorking({ + agentType: 'codex', + previousState: 'working', + incomingState: 'working', + previousPrompt: 'first', + incomingPrompt: 'second', + hasExplicitPrompt: true + }) + ).toBe(false) + }) +}) diff --git a/src/shared/command-code-turn-boundary.ts b/src/shared/command-code-turn-boundary.ts new file mode 100644 index 00000000000..6120f55a436 --- /dev/null +++ b/src/shared/command-code-turn-boundary.ts @@ -0,0 +1,56 @@ +import type { AgentType } from './agent-status-types' + +type CommandCodeTurnBoundaryInput = { + agentType: AgentType | undefined + previousState?: string + incomingState: string + previousPrompt?: string + incomingPrompt: string + hasExplicitPrompt?: boolean + previousPromptInteractionKey?: string + incomingPromptInteractionKey?: string +} + +/** Command Code has no UserPromptSubmit hook; a new transcript prompt is the turn boundary. */ +export function isCommandCodeNewTurnWhileWorking({ + agentType, + previousState, + incomingState, + previousPrompt, + incomingPrompt, + hasExplicitPrompt, + previousPromptInteractionKey, + incomingPromptInteractionKey +}: CommandCodeTurnBoundaryInput): boolean { + if (agentType !== 'command-code') { + return false + } + if (previousState !== 'working' || incomingState !== 'working') { + return false + } + + const nextPrompt = incomingPrompt.trim() + if (nextPrompt.length === 0) { + return false + } + + if ( + incomingPromptInteractionKey !== undefined && + previousPromptInteractionKey !== undefined && + incomingPromptInteractionKey !== previousPromptInteractionKey + ) { + return true + } + + if (nextPrompt === (previousPrompt ?? '').trim()) { + return false + } + + // Main-process hooks can require explicit prompt evidence; renderer IPC does + // not carry the flag yet, so a prompt text change is enough there. + if (hasExplicitPrompt === false) { + return false + } + + return true +} From 00fc452c2142abd95a100d608b31852be7525242 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 12 Jul 2026 18:07:06 -0700 Subject: [PATCH 2/4] Split client-ui RPC schemas into client-ui-schemas.ts (#8446) #7598 pushed client-ui.ts to 301 lines, one over the max-lines limit, which breaks repo lint for every PR that merges current main (verify does not run on main pushes). Follow the existing browser-schemas.ts/computer-schemas.ts pattern: zod schemas move to a sibling module, the method registry stays. Co-authored-by: Brennan Benson <> --- .../runtime/rpc/methods/client-ui-schemas.ts | 267 +++++++++++++++++ src/main/runtime/rpc/methods/client-ui.ts | 276 +----------------- 2 files changed, 277 insertions(+), 266 deletions(-) create mode 100644 src/main/runtime/rpc/methods/client-ui-schemas.ts diff --git a/src/main/runtime/rpc/methods/client-ui-schemas.ts b/src/main/runtime/rpc/methods/client-ui-schemas.ts new file mode 100644 index 00000000000..5c1823a4e8e --- /dev/null +++ b/src/main/runtime/rpc/methods/client-ui-schemas.ts @@ -0,0 +1,267 @@ +import { z } from 'zod' +import { + isFeatureInteractionId, + type FeatureInteractionId +} from '../../../../shared/feature-interactions' +import { isFeatureTipId } from '../../../../shared/feature-tips' +import { + normalizeTuiAgentArgsRecord, + normalizeTuiAgentEnvRecord +} from '../../../../shared/tui-agent-launch-defaults' +import { isTuiAgent } from '../../../../shared/tui-agent-config' +import { isTaskProvider } from '../../../../shared/task-providers' +import { normalizeDisabledTuiAgents } from '../../../../shared/tui-agent-selection' +import { normalizePRBotAuthorOverrides } from '../../../../shared/pr-bot-author-overrides' +import { normalizeWorktreeCardProperties } from '../../../../shared/worktree-card-properties' +import type { TaskProvider } from '../../../../shared/types' + +const NullableString = z.string().nullable() +const StringArray = z.array(z.string()) +const TaskProviderParam = z.custom(isTaskProvider, { + message: 'Unknown task provider' +}) +const FeatureTipIds = z.array(z.custom(isFeatureTipId, { message: 'Unknown feature tip id' })) +const UnknownRecord = z.record(z.string(), z.unknown()) +const UnknownRecordArray = z.array(UnknownRecord) +const LegacyWorktreeCardProperty = z.enum([ + 'status', + 'unread', + 'ci', + 'branch', + 'issue', + 'linear-issue', + 'pr', + 'automation', + 'comment', + 'ports', + 'inline-agents' +]) +const WorktreeCardProperties = z + .array(LegacyWorktreeCardProperty) + .transform((value) => normalizeWorktreeCardProperties(value)) +const AgentActivityDisplayMode = z.enum(['compact', 'full']) +const StatusBarItem = z.enum([ + 'claude', + 'codex', + 'gemini', + 'antigravity', + 'opencode-go', + 'kimi', + 'minimax', + 'grok', + 'ssh', + 'resource-usage', + 'ports' +]) +const WorkspaceStatusDefinition = z.object({ + id: z.string(), + label: z.string(), + color: z.string().optional(), + icon: z.string().optional() +}) +const TaskResumeState = z + .object({ + githubMode: z.enum(['items', 'project']).optional(), + githubItemsPreset: z.string().nullable().optional(), + githubItemsQuery: z.string().optional(), + githubProjectHiddenFieldIdsByView: z.record(z.string(), z.array(z.string())).optional(), + linearPreset: z.enum(['assigned', 'created', 'all', 'completed']).optional(), + linearQuery: z.string().optional() + }) + .strict() +const WorkspaceCleanupDismissal = z + .object({ + worktreeId: z.string(), + dismissedAt: z.number().finite(), + fingerprint: z.string(), + classifierVersion: z.number().finite() + }) + .strict() +const WorkspaceCleanup = z + .object({ + dismissals: z.record(z.string(), WorkspaceCleanupDismissal) + }) + .strict() +const FeatureInteractionRecord = z + .object({ + firstInteractedAt: z.number().finite().nonnegative(), + interactionCount: z.number().int().positive().optional() + }) + .strict() +const FeatureInteractions = z + .record(z.string(), FeatureInteractionRecord) + .superRefine((value, ctx) => { + for (const id of Object.keys(value)) { + if (!isFeatureInteractionId(id)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `Unknown feature interaction id: ${id}`, + path: [id] + }) + } + } + }) +export const FeatureInteractionIdParam = z.custom(isFeatureInteractionId, { + message: 'Unknown feature interaction id' +}) +export const PRBotAuthorOverrideUpdate = z + .object({ author: z.string(), isBot: z.boolean() }) + .strict() +const GitHubProjectRef = z + .object({ + owner: z.string(), + ownerType: z.enum(['organization', 'user']), + number: z.number().int() + }) + .strict() +const GitHubProjectSettings = z + .object({ + pinned: z.array(GitHubProjectRef), + recent: z.array( + GitHubProjectRef.extend({ + lastOpenedAt: z.string() + }).strict() + ), + lastViewByProject: z.record(z.string(), z.object({ viewId: z.string() }).strict()), + activeProject: GitHubProjectRef.nullable() + }) + .strict() + +export const SettingsUpdate = z + .object({ + defaultTuiAgent: z + .unknown() + .transform((value) => + value === null || value === 'blank' || isTuiAgent(value) ? value : undefined + ) + .optional(), + disabledTuiAgents: z + .unknown() + .transform((value) => normalizeDisabledTuiAgents(value)) + .optional(), + agentDefaultArgs: z + .unknown() + .transform((value) => normalizeTuiAgentArgsRecord(value)) + .optional(), + agentDefaultEnv: z + .unknown() + .transform((value) => normalizeTuiAgentEnvRecord(value)) + .optional(), + defaultTaskSource: TaskProviderParam.optional(), + visibleTaskProviders: z.array(TaskProviderParam).optional(), + defaultTaskViewPreset: z + .enum(['issues', 'my-issues', 'prs', 'my-prs', 'review', 'all']) + .optional(), + experimentalNewWorktreeCardStyle: z.boolean().optional(), + agentStatusHooksEnabled: z.boolean().optional(), + defaultRepoSelection: z.array(z.string()).nullable().optional(), + defaultLinearTeamSelection: z.array(z.string()).nullable().optional(), + compactWorktreeCards: z.boolean().optional(), + minimaxGroupId: z.string().optional(), + minimaxUsageModels: z.string().optional(), + githubProjects: GitHubProjectSettings.optional(), + prBotAuthorOverrides: z + .unknown() + .transform((value) => normalizePRBotAuthorOverrides(value)) + .optional() + }) + .strict() + .default({}) + +export const UiUpdate = z + .object({ + lastActiveRepoId: NullableString.optional(), + lastActiveWorktreeId: NullableString.optional(), + sidebarWidth: z.number().finite().optional(), + rightSidebarOpen: z.boolean().optional(), + rightSidebarTab: z + .enum(['explorer', 'search', 'vault', 'source-control', 'checks', 'ports']) + .optional(), + rightSidebarExplorerView: z.enum(['files', 'search']).optional(), + rightSidebarWidth: z.number().finite().optional(), + markdownTocPanelWidth: z.number().finite().optional(), + groupBy: z.enum(['none', 'workspace-status', 'repo', 'pr-status']).optional(), + showWorkspaceLineage: z.boolean().optional(), + sortBy: z.enum(['name', 'smart', 'recent', 'repo', 'manual']).optional(), + projectOrderBy: z.enum(['manual', 'recent']).optional(), + showActiveOnly: z.boolean().optional(), + hideSleepingWorkspaces: z.boolean().optional(), + showSleepingWorkspaces: z.boolean().optional(), + showInactiveWorkspaces: z.boolean().optional(), + workspaceHostScope: z.string().optional(), + visibleWorkspaceHostIds: z.array(z.string()).nullable().optional(), + workspaceHostOrder: z.array(z.string()).optional(), + hideDefaultBranchWorkspace: z.boolean().optional(), + hideAutomationGeneratedWorkspaces: z.boolean().optional(), + filterRepoIds: StringArray.optional(), + collapsedGroups: StringArray.optional(), + uiZoomLevel: z.number().finite().optional(), + editorFontZoomLevel: z.number().finite().optional(), + worktreeCardProperties: WorktreeCardProperties.optional(), + _worktreeCardModeDefaulted: z.boolean().optional(), + agentActivityDisplayMode: AgentActivityDisplayMode.optional(), + workspaceStatuses: z.array(WorkspaceStatusDefinition).optional(), + workspaceBoardOpacity: z.number().finite().optional(), + workspaceBoardColumnWidth: z.number().finite().optional(), + syncTaskStatusFromWorkspaceBoard: z.boolean().optional(), + _workspaceStatusesDefaultOrderMigrated: z.boolean().optional(), + _workspaceStatusesReorderedDefaultRepaired: z.boolean().optional(), + _workspaceStatusesDefaultWorkflowMigrated: z.boolean().optional(), + _workspaceStatusesDefaultVisualsMigrated: z.boolean().optional(), + statusBarItems: z.array(StatusBarItem).optional(), + _portsStatusBarDefaultAdded: z.boolean().optional(), + _kimiStatusBarDefaultAdded: z.boolean().optional(), + _minimaxStatusBarDefaultAdded: z.boolean().optional(), + _antigravityStatusBarDefaultAdded: z.boolean().optional(), + _grokStatusBarDefaultAdded: z.boolean().optional(), + statusBarVisible: z.boolean().optional(), + usagePercentageDisplay: z.enum(['used', 'remaining']).optional(), + dismissedUpdateVersion: NullableString.optional(), + lastUpdateCheckAt: z.number().finite().nullable().optional(), + pendingUpdateNudgeId: NullableString.optional(), + dismissedUpdateNudgeId: NullableString.optional(), + notificationPermissionRequested: z.boolean().optional(), + updateReassuranceSeen: z.boolean().optional(), + acknowledgedAgentsByPaneKey: z.record(z.string(), z.number().finite()).optional(), + browserDefaultUrl: NullableString.optional(), + browserDefaultSearchEngine: z + .enum(['google', 'duckduckgo', 'bing', 'kagi']) + .nullable() + .optional(), + browserDefaultZoomLevel: z.number().finite().optional(), + browserKagiSessionLink: NullableString.optional(), + windowBounds: z + .object({ + x: z.number().finite(), + y: z.number().finite(), + width: z.number().finite(), + height: z.number().finite() + }) + .nullable() + .optional(), + windowMaximized: z.boolean().optional(), + _sortBySmartMigrated: z.boolean().optional(), + _inlineAgentsDefaultedForExperiment: z.boolean().optional(), + _inlineAgentsDefaultedForAllUsers: z.boolean().optional(), + trustedOrcaHooks: z.record(z.string(), z.unknown()).optional(), + setupScriptPromptDismissedRepoIds: StringArray.optional(), + projectOrderManualDefaultNoticeDismissed: z.boolean().optional(), + usagePercentageDisplayChangeNoticeDismissed: z.boolean().optional(), + usageEmptyStateDismissed: z.boolean().optional(), + petVisible: z.boolean().optional(), + petId: z.string().optional(), + customPets: UnknownRecordArray.optional(), + petSize: z.number().finite().optional(), + sidekickVisible: z.boolean().optional(), + sidekickId: z.string().optional(), + customSidekicks: UnknownRecordArray.optional(), + sidekickSize: z.number().finite().optional(), + taskResumeState: TaskResumeState.optional(), + workspaceCleanup: WorkspaceCleanup.optional(), + featureTipsSeenIds: FeatureTipIds.optional(), + featureInteractions: FeatureInteractions.optional(), + contextualToursSeenIds: StringArray.optional(), + contextualToursAutoEligible: z.boolean().optional() + }) + .strict() + .default({}) diff --git a/src/main/runtime/rpc/methods/client-ui.ts b/src/main/runtime/rpc/methods/client-ui.ts index 241824e43a5..7945203f958 100644 --- a/src/main/runtime/rpc/methods/client-ui.ts +++ b/src/main/runtime/rpc/methods/client-ui.ts @@ -1,269 +1,11 @@ -import { z } from 'zod' -import { - isFeatureInteractionId, - type FeatureInteractionId -} from '../../../../shared/feature-interactions' -import { isFeatureTipId } from '../../../../shared/feature-tips' -import { - normalizeTuiAgentArgsRecord, - normalizeTuiAgentEnvRecord -} from '../../../../shared/tui-agent-launch-defaults' -import { isTuiAgent } from '../../../../shared/tui-agent-config' -import { isTaskProvider } from '../../../../shared/task-providers' -import { normalizeDisabledTuiAgents } from '../../../../shared/tui-agent-selection' -import { normalizePRBotAuthorOverrides } from '../../../../shared/pr-bot-author-overrides' -import { normalizeWorktreeCardProperties } from '../../../../shared/worktree-card-properties' -import type { PersistedUIState, TaskProvider } from '../../../../shared/types' +import type { PersistedUIState } from '../../../../shared/types' import { defineMethod, type RpcMethod } from '../core' - -const NullableString = z.string().nullable() -const StringArray = z.array(z.string()) -const TaskProviderParam = z.custom(isTaskProvider, { - message: 'Unknown task provider' -}) -const FeatureTipIds = z.array(z.custom(isFeatureTipId, { message: 'Unknown feature tip id' })) -const UnknownRecord = z.record(z.string(), z.unknown()) -const UnknownRecordArray = z.array(UnknownRecord) -const LegacyWorktreeCardProperty = z.enum([ - 'status', - 'unread', - 'ci', - 'branch', - 'issue', - 'linear-issue', - 'pr', - 'automation', - 'comment', - 'ports', - 'inline-agents' -]) -const WorktreeCardProperties = z - .array(LegacyWorktreeCardProperty) - .transform((value) => normalizeWorktreeCardProperties(value)) -const AgentActivityDisplayMode = z.enum(['compact', 'full']) -const StatusBarItem = z.enum([ - 'claude', - 'codex', - 'gemini', - 'antigravity', - 'opencode-go', - 'kimi', - 'minimax', - 'grok', - 'ssh', - 'resource-usage', - 'ports' -]) -const WorkspaceStatusDefinition = z.object({ - id: z.string(), - label: z.string(), - color: z.string().optional(), - icon: z.string().optional() -}) -const TaskResumeState = z - .object({ - githubMode: z.enum(['items', 'project']).optional(), - githubItemsPreset: z.string().nullable().optional(), - githubItemsQuery: z.string().optional(), - githubProjectHiddenFieldIdsByView: z.record(z.string(), z.array(z.string())).optional(), - linearPreset: z.enum(['assigned', 'created', 'all', 'completed']).optional(), - linearQuery: z.string().optional() - }) - .strict() -const WorkspaceCleanupDismissal = z - .object({ - worktreeId: z.string(), - dismissedAt: z.number().finite(), - fingerprint: z.string(), - classifierVersion: z.number().finite() - }) - .strict() -const WorkspaceCleanup = z - .object({ - dismissals: z.record(z.string(), WorkspaceCleanupDismissal) - }) - .strict() -const FeatureInteractionRecord = z - .object({ - firstInteractedAt: z.number().finite().nonnegative(), - interactionCount: z.number().int().positive().optional() - }) - .strict() -const FeatureInteractions = z - .record(z.string(), FeatureInteractionRecord) - .superRefine((value, ctx) => { - for (const id of Object.keys(value)) { - if (!isFeatureInteractionId(id)) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: `Unknown feature interaction id: ${id}`, - path: [id] - }) - } - } - }) -const FeatureInteractionIdParam = z.custom(isFeatureInteractionId, { - message: 'Unknown feature interaction id' -}) -const PRBotAuthorOverrideUpdate = z.object({ author: z.string(), isBot: z.boolean() }).strict() -const GitHubProjectRef = z - .object({ - owner: z.string(), - ownerType: z.enum(['organization', 'user']), - number: z.number().int() - }) - .strict() -const GitHubProjectSettings = z - .object({ - pinned: z.array(GitHubProjectRef), - recent: z.array( - GitHubProjectRef.extend({ - lastOpenedAt: z.string() - }).strict() - ), - lastViewByProject: z.record(z.string(), z.object({ viewId: z.string() }).strict()), - activeProject: GitHubProjectRef.nullable() - }) - .strict() - -const SettingsUpdate = z - .object({ - defaultTuiAgent: z - .unknown() - .transform((value) => - value === null || value === 'blank' || isTuiAgent(value) ? value : undefined - ) - .optional(), - disabledTuiAgents: z - .unknown() - .transform((value) => normalizeDisabledTuiAgents(value)) - .optional(), - agentDefaultArgs: z - .unknown() - .transform((value) => normalizeTuiAgentArgsRecord(value)) - .optional(), - agentDefaultEnv: z - .unknown() - .transform((value) => normalizeTuiAgentEnvRecord(value)) - .optional(), - defaultTaskSource: TaskProviderParam.optional(), - visibleTaskProviders: z.array(TaskProviderParam).optional(), - defaultTaskViewPreset: z - .enum(['issues', 'my-issues', 'prs', 'my-prs', 'review', 'all']) - .optional(), - experimentalNewWorktreeCardStyle: z.boolean().optional(), - agentStatusHooksEnabled: z.boolean().optional(), - defaultRepoSelection: z.array(z.string()).nullable().optional(), - defaultLinearTeamSelection: z.array(z.string()).nullable().optional(), - compactWorktreeCards: z.boolean().optional(), - minimaxGroupId: z.string().optional(), - minimaxUsageModels: z.string().optional(), - githubProjects: GitHubProjectSettings.optional(), - prBotAuthorOverrides: z - .unknown() - .transform((value) => normalizePRBotAuthorOverrides(value)) - .optional() - }) - .strict() - .default({}) - -const UiUpdate = z - .object({ - lastActiveRepoId: NullableString.optional(), - lastActiveWorktreeId: NullableString.optional(), - sidebarWidth: z.number().finite().optional(), - rightSidebarOpen: z.boolean().optional(), - rightSidebarTab: z - .enum(['explorer', 'search', 'vault', 'source-control', 'checks', 'ports']) - .optional(), - rightSidebarExplorerView: z.enum(['files', 'search']).optional(), - rightSidebarWidth: z.number().finite().optional(), - markdownTocPanelWidth: z.number().finite().optional(), - groupBy: z.enum(['none', 'workspace-status', 'repo', 'pr-status']).optional(), - showWorkspaceLineage: z.boolean().optional(), - sortBy: z.enum(['name', 'smart', 'recent', 'repo', 'manual']).optional(), - projectOrderBy: z.enum(['manual', 'recent']).optional(), - showActiveOnly: z.boolean().optional(), - hideSleepingWorkspaces: z.boolean().optional(), - showSleepingWorkspaces: z.boolean().optional(), - showInactiveWorkspaces: z.boolean().optional(), - workspaceHostScope: z.string().optional(), - visibleWorkspaceHostIds: z.array(z.string()).nullable().optional(), - workspaceHostOrder: z.array(z.string()).optional(), - hideDefaultBranchWorkspace: z.boolean().optional(), - hideAutomationGeneratedWorkspaces: z.boolean().optional(), - filterRepoIds: StringArray.optional(), - collapsedGroups: StringArray.optional(), - uiZoomLevel: z.number().finite().optional(), - editorFontZoomLevel: z.number().finite().optional(), - worktreeCardProperties: WorktreeCardProperties.optional(), - _worktreeCardModeDefaulted: z.boolean().optional(), - agentActivityDisplayMode: AgentActivityDisplayMode.optional(), - workspaceStatuses: z.array(WorkspaceStatusDefinition).optional(), - workspaceBoardOpacity: z.number().finite().optional(), - workspaceBoardColumnWidth: z.number().finite().optional(), - syncTaskStatusFromWorkspaceBoard: z.boolean().optional(), - _workspaceStatusesDefaultOrderMigrated: z.boolean().optional(), - _workspaceStatusesReorderedDefaultRepaired: z.boolean().optional(), - _workspaceStatusesDefaultWorkflowMigrated: z.boolean().optional(), - _workspaceStatusesDefaultVisualsMigrated: z.boolean().optional(), - statusBarItems: z.array(StatusBarItem).optional(), - _portsStatusBarDefaultAdded: z.boolean().optional(), - _kimiStatusBarDefaultAdded: z.boolean().optional(), - _minimaxStatusBarDefaultAdded: z.boolean().optional(), - _antigravityStatusBarDefaultAdded: z.boolean().optional(), - _grokStatusBarDefaultAdded: z.boolean().optional(), - statusBarVisible: z.boolean().optional(), - usagePercentageDisplay: z.enum(['used', 'remaining']).optional(), - dismissedUpdateVersion: NullableString.optional(), - lastUpdateCheckAt: z.number().finite().nullable().optional(), - pendingUpdateNudgeId: NullableString.optional(), - dismissedUpdateNudgeId: NullableString.optional(), - notificationPermissionRequested: z.boolean().optional(), - updateReassuranceSeen: z.boolean().optional(), - acknowledgedAgentsByPaneKey: z.record(z.string(), z.number().finite()).optional(), - browserDefaultUrl: NullableString.optional(), - browserDefaultSearchEngine: z - .enum(['google', 'duckduckgo', 'bing', 'kagi']) - .nullable() - .optional(), - browserDefaultZoomLevel: z.number().finite().optional(), - browserKagiSessionLink: NullableString.optional(), - windowBounds: z - .object({ - x: z.number().finite(), - y: z.number().finite(), - width: z.number().finite(), - height: z.number().finite() - }) - .nullable() - .optional(), - windowMaximized: z.boolean().optional(), - _sortBySmartMigrated: z.boolean().optional(), - _inlineAgentsDefaultedForExperiment: z.boolean().optional(), - _inlineAgentsDefaultedForAllUsers: z.boolean().optional(), - trustedOrcaHooks: z.record(z.string(), z.unknown()).optional(), - setupScriptPromptDismissedRepoIds: StringArray.optional(), - projectOrderManualDefaultNoticeDismissed: z.boolean().optional(), - usagePercentageDisplayChangeNoticeDismissed: z.boolean().optional(), - usageEmptyStateDismissed: z.boolean().optional(), - petVisible: z.boolean().optional(), - petId: z.string().optional(), - customPets: UnknownRecordArray.optional(), - petSize: z.number().finite().optional(), - sidekickVisible: z.boolean().optional(), - sidekickId: z.string().optional(), - customSidekicks: UnknownRecordArray.optional(), - sidekickSize: z.number().finite().optional(), - taskResumeState: TaskResumeState.optional(), - workspaceCleanup: WorkspaceCleanup.optional(), - featureTipsSeenIds: FeatureTipIds.optional(), - featureInteractions: FeatureInteractions.optional(), - contextualToursSeenIds: StringArray.optional(), - contextualToursAutoEligible: z.boolean().optional() - }) - .strict() - .default({}) +import { + FeatureInteractionIdParam, + PRBotAuthorOverrideUpdate, + SettingsUpdate, + UiUpdate +} from './client-ui-schemas' export const CLIENT_UI_METHODS: RpcMethod[] = [ defineMethod({ @@ -279,7 +21,9 @@ export const CLIENT_UI_METHODS: RpcMethod[] = [ defineMethod({ name: 'settings.updatePRBotAuthorOverride', params: PRBotAuthorOverrideUpdate, - handler: (params, { runtime }) => ({ settings: runtime.updateClientPRBotAuthorOverride(params) }) + handler: (params, { runtime }) => ({ + settings: runtime.updateClientPRBotAuthorOverride(params) + }) }), defineMethod({ name: 'ui.get', From 433df4be3c4b2c5ed03589be48690b92693b51e5 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 12 Jul 2026 18:15:15 -0700 Subject: [PATCH 3/4] fix(runtime): prevent file watcher SIGSEGV from crashing orca serve (#8370) * Fix crash-isolated file watcher process pool for orca-serve SIGSEGV afte Replace the worker-thread runtime file watcher with a forked, crash-isolated @parcel/watcher child process pool so a native FSEvents fault can no longer take down the main/serve process, and add bounded event batching, delivery backpressure, and quarantine-based recovery for faulty watch roots. * Fix crash-isolated file watcher teardown and shutdown leaks - Fault harness could throw before mkdtemp/realpath completed, skipping cleanup; now tracks each temp path independently and races an async watcher-callback error so it can't escape the try/finally unhandled. - In-process fallback swallowed unsubscribe failures via a bare rejection handler that could still throw; use .catch() instead. - Watcher process entry's cancel-subscribe handler now reuses the async unsubscribe path when a crawl already finished, releasing the native handle instead of leaking it (blocks worktree unlock on Windows). - Runtime watcher process pool exposed no real dispose(); shutdown now kills pooled children so they don't outlive the main process. * Fix disposeSlot double-iteration bug in file watcher pool teardown Remove the unnecessary array snapshot in dispose(): disposeSlot mutates allSlots by deleting the slot being visited, and deleting the in-progress element during Set iteration is well-defined, so the spread copy was dead weight left over from prior debugging. * Fix pending file watcher installs not aborting on unsubscribe - Local/WSL watcher installs and SSH fs.watch setup now honor the in-flight AbortSignal, so the last unwatch cancels a slow native subscribe or remote setup instead of waiting for it to finish. - Thread signal through IFilesystemProvider.watch and SSH-backed file explorer watches for the same early-cancel behavior. * Fix crash-resubscribe hangs and SSH watch teardown races in file watcher - Add a bounded deadline for post-crash resubscription crawls so one stuck root quarantines instead of pinning its whole shard forever. - Report FSEvents overflow as recoverable so delivery continues after a dropped-events error instead of surfacing as terminal. - Make WSL watcher abort errors real DOMException instances so AbortSignal-based cancellation checks recognize them. - Rework SSH watch registration so ownership of the shared setup request (not just the first caller) decides teardown, preventing one caller's abort from cancelling another's shared watch and guaranteeing exactly one fs.unwatch per registration. - Reformat reliability-gates.jsonc arrays and refresh WSL/SSH coverage entries and evidence runs to match the above. * Add CI gate to run the file-watcher SIGSEGV fault harness under Electron - The reliability gate and release workflows (mac, Linux) previously only exercised the crash-isolation harness under vanilla Node, which doesn't catch runtime differences in the actual Electron binary that ships to users. - Adds an `ELECTRON_RUN_AS_NODE=1 pnpm exec electron ...` run of the same harness alongside the existing Node run, so #8212's SIGSEGV-survival contract is proven against both runtimes before packaging. * Add CI gate blocking Linux/macOS release packaging on watcher fault reco Adds a contract test asserting release-cut.yml and release-mac-build.yml run the runtime-file-watcher-fault-harness after building and before publishing artifacts, so a regression in watcher process fault recovery fails release packaging instead of shipping silently. * Fix use-after-clear crash in failAllWatcherSubscriptions Snapshot the records map before iterating, since onTerminalError hooks can dispose the supervisor and clear `records` mid-loop, causing a crash. Also update the matching test to assert against the shared buildParcelWatcherIgnoreOptions helper instead of a loose arrayContaining match. * Fix use-after-clear crash in failAllWatcherSubscriptions Snapshot watcher records with Array.from instead of spread, since spread syntax over an iterator that's mutated mid-loop by onTerminalError hooks can produce inconsistent results. --- .github/workflows/release-cut.yml | 8 + .github/workflows/release-mac-build.yml | 7 + config/reliability-gates.jsonc | 213 ++++++++ ...package-electron-runtime-contract.test.mjs | 31 ++ .../runtime-file-watcher-fault-harness.mjs | 180 +++++++ electron.vite.config.ts | 1 - ...lesystem-watcher-local-unsubscribe.test.ts | 83 +++ src/main/ipc/filesystem-watcher-wsl.test.ts | 22 + src/main/ipc/filesystem-watcher-wsl.ts | 22 +- src/main/ipc/filesystem-watcher.ts | 60 ++- .../ipc/parcel-watcher-canary-directory.ts | 26 + src/main/ipc/parcel-watcher-child-launch.ts | 46 ++ src/main/ipc/parcel-watcher-crash-fuse.ts | 24 + .../ipc/parcel-watcher-event-delivery.test.ts | 44 ++ src/main/ipc/parcel-watcher-event-delivery.ts | 160 ++++++ .../ipc/parcel-watcher-host-subscriptions.ts | 140 +++++ .../ipc/parcel-watcher-in-process-fallback.ts | 124 +++++ .../ipc/parcel-watcher-pending-subscribe.ts | 105 ++++ .../ipc/parcel-watcher-process-entry.test.ts | 276 +++++++++- src/main/ipc/parcel-watcher-process-entry.ts | 131 +++-- .../ipc/parcel-watcher-process-failure.ts | 25 + .../ipc/parcel-watcher-process-protocol.ts | 37 ++ .../parcel-watcher-process-subscription.ts | 35 ++ .../ipc/parcel-watcher-process-supervisor.ts | 316 ++++++++++++ src/main/ipc/parcel-watcher-process.test.ts | 411 ++++++++++++++- src/main/ipc/parcel-watcher-process.ts | 368 +++----------- .../parcel-watcher-supervisor-subscribe.ts | 127 +++++ .../ipc/runtime-watcher-process-pool.test.ts | 273 ++++++++++ src/main/ipc/runtime-watcher-process-pool.ts | 251 +++++++++ .../ssh-filesystem-provider-watch.ts | 178 ++++++- .../providers/ssh-filesystem-provider.test.ts | 97 +++- src/main/providers/ssh-filesystem-provider.ts | 15 +- src/main/providers/types.ts | 6 +- src/main/runtime/file-watcher-host.test.ts | 477 ++++++++++-------- src/main/runtime/file-watcher-host.ts | 406 ++++++++++----- src/main/runtime/file-watcher-worker.ts | 141 ------ .../runtime/orca-runtime-files-watch.test.ts | 108 +++- src/main/runtime/orca-runtime-files.test.ts | 19 +- src/main/runtime/orca-runtime-files.ts | 21 +- src/main/runtime/rpc/methods/files.test.ts | 64 ++- src/main/runtime/rpc/methods/files.ts | 41 +- .../src/runtime/runtime-file-client.test.ts | 109 ++++ .../src/runtime/runtime-file-client.ts | 28 +- 43 files changed, 4311 insertions(+), 945 deletions(-) create mode 100644 config/scripts/runtime-file-watcher-fault-harness.mjs create mode 100644 src/main/ipc/parcel-watcher-canary-directory.ts create mode 100644 src/main/ipc/parcel-watcher-child-launch.ts create mode 100644 src/main/ipc/parcel-watcher-crash-fuse.ts create mode 100644 src/main/ipc/parcel-watcher-event-delivery.test.ts create mode 100644 src/main/ipc/parcel-watcher-event-delivery.ts create mode 100644 src/main/ipc/parcel-watcher-host-subscriptions.ts create mode 100644 src/main/ipc/parcel-watcher-in-process-fallback.ts create mode 100644 src/main/ipc/parcel-watcher-pending-subscribe.ts create mode 100644 src/main/ipc/parcel-watcher-process-failure.ts create mode 100644 src/main/ipc/parcel-watcher-process-protocol.ts create mode 100644 src/main/ipc/parcel-watcher-process-subscription.ts create mode 100644 src/main/ipc/parcel-watcher-process-supervisor.ts create mode 100644 src/main/ipc/parcel-watcher-supervisor-subscribe.ts create mode 100644 src/main/ipc/runtime-watcher-process-pool.test.ts create mode 100644 src/main/ipc/runtime-watcher-process-pool.ts delete mode 100644 src/main/runtime/file-watcher-worker.ts diff --git a/.github/workflows/release-cut.yml b/.github/workflows/release-cut.yml index 397772bfb7e..b9d3dea8629 100644 --- a/.github/workflows/release-cut.yml +++ b/.github/workflows/release-cut.yml @@ -863,6 +863,14 @@ jobs: ORCA_DIAGNOSTICS_TOKEN_URL: https://www.onorca.dev/diagnostics/token ORCA_POSTHOG_WRITE_KEY: ${{ secrets.ORCA_POSTHOG_WRITE_KEY }} + - name: Gate runtime file-watcher process isolation + if: runner.os == 'Linux' + run: | + # Why: #8212 is a native-process crash contract. Prove both the Node + # host and the exact Electron runtime survive SIGSEGV before packaging. + node config/scripts/runtime-file-watcher-fault-harness.mjs + ELECTRON_RUN_AS_NODE=1 pnpm exec electron config/scripts/runtime-file-watcher-fault-harness.mjs + - name: Publish release artifacts (Linux) if: matrix.platform == 'linux-x64' || matrix.platform == 'linux-arm64' uses: nick-fields/retry@v4 diff --git a/.github/workflows/release-mac-build.yml b/.github/workflows/release-mac-build.yml index 3638d40f00c..d08584c1077 100644 --- a/.github/workflows/release-mac-build.yml +++ b/.github/workflows/release-mac-build.yml @@ -120,6 +120,13 @@ jobs: ORCA_DIAGNOSTICS_TOKEN_URL: https://www.onorca.dev/diagnostics/token ORCA_POSTHOG_WRITE_KEY: ${{ secrets.ORCA_POSTHOG_WRITE_KEY }} + - name: Gate runtime file-watcher process isolation + run: | + # Why: #8212 is a native-process crash contract. Prove both the Node + # host and the exact Electron runtime survive SIGSEGV before packaging. + node config/scripts/runtime-file-watcher-fault-harness.mjs + ELECTRON_RUN_AS_NODE=1 pnpm exec electron config/scripts/runtime-file-watcher-fault-harness.mjs + - name: Publish release artifacts (macOS) uses: nick-fields/retry@v4 with: diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 7133fd9fb56..542fe821868 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -3747,6 +3747,219 @@ "No live Electron reload-survival assertion yet — the gate proves the main-side accounting reset, not the full renderer reattach paint." ], "demotionRule": "Demote or quarantine if the gate flakes once without a product bug or harness bug filed to the owner." + }, + { + "id": "runtime-files.watcher-process-isolation", + "title": "Runtime file watcher faults stay process-isolated and recover without host starvation", + "maturity": "experimental", + "protection": "partial", + "owner": "runtime-platform", + "layer": "process-rpc-contract", + "surfaces": [ + "headless orca serve", + "paired file explorer", + "runtime editor external-change detection", + "desktop filesystem watcher" + ], + "platforms": [ + "macos", + "linux", + "windows" + ], + "providers": [ + "local", + "remote-runtime", + "ssh" + ], + "coveredPlatforms": [ + "macos" + ], + "coveredProviders": [ + "local", + "remote-runtime", + "ssh" + ], + "coverageNotes": "Deterministic tests cover a four-child healthy runtime pool plus at most four bounded quarantine children, one quarantine attempt per watch lifetime, queue-aware cancellation, bounded initial and crash-resubscribe crawls, physical desktop-install abort, stale-generation rejection, per-child stat bounds, final-overflow RPC delivery, and renderer eviction before terminal callbacks can retry. A built-entry macOS harness drives the real supervisor, kills its child with SIGSEGV, then requires automatic resubscription and a later event. Windows keeps the existing fs.watch runtime path; SSH shared-watch setup uses registration-owned cancellation so one caller cannot cancel another caller's watch.", + "motivatingLinks": [ + "https://github.com/stablyai/orca/issues/5308", + "https://github.com/stablyai/orca/issues/7547", + "https://github.com/stablyai/orca/issues/8212" + ], + "invariant": "Local runtime roots use at most four shared watcher children while healthy and at most four additional quarantine children after shard failure. Cancelling queued setup removes only that request; cancelling an active crawl restarts one shard and restores its healthy records. Setup deadlines start only when the child owns the crawl; the runtime RPC transport owns the normal 15-second initial budget, while a 60-second recovery deadline quarantines a root that would otherwise pin its shard. A crash-fused shared shard moves affected roots into the bounded quarantine pool, and a second fused quarantine ends those watch lifetimes instead of spawning replacements indefinitely. Every interrupted setup emits overflow after recovery, directory metadata work stays bounded to eight concurrent operations per child, destructive desktop cleanup physically aborts pending native setup, and terminal renderer watches are evicted before error callbacks can retry.", + "oracle": "Fill the four-child healthy pool with multiple roots, inject IPC disconnect-before-exit, queued and active setup cancellation, initial and crash-resubscribe timeouts, and shard-level terminal failure, then assert healthy roots are restored, affected roots spread across no more than four quarantine supervisors, a second quarantine fuse creates no replacement, stale generations stop delivering, and existing same-root clients receive overflow plus later events without error/end. Abort a pending desktop install and require worktree close and app shutdown to settle before the native crawl resolves. Synchronously retry from a renderer terminal-error callback and require a fresh stream that survives the old stream's end. Run eight simultaneous metadata batches in one child and assert no more than eight stats are active there. Drive a real built child through the source supervisor, kill it with SIGSEGV, and require automatic resubscription plus a post-crash event while the harness parent remains alive.", + "commands": [ + "pnpm exec vitest run --config config/vitest.config.ts src/main/ipc/parcel-watcher-process.test.ts src/main/ipc/runtime-watcher-process-pool.test.ts src/main/ipc/parcel-watcher-process-entry.test.ts src/main/ipc/parcel-watcher-event-delivery.test.ts src/main/ipc/filesystem-watcher-local-unsubscribe.test.ts src/main/ipc/filesystem-watcher-wsl.test.ts src/main/runtime/file-watcher-host.test.ts src/main/runtime/rpc/methods/files.test.ts src/main/providers/ssh-filesystem-provider.test.ts src/renderer/src/runtime/runtime-file-client.test.ts", + "pnpm run build:electron-vite", + "node config/scripts/runtime-file-watcher-fault-harness.mjs", + "ELECTRON_RUN_AS_NODE=1 pnpm exec electron config/scripts/runtime-file-watcher-fault-harness.mjs" + ], + "testFiles": [ + "src/main/ipc/parcel-watcher-process.test.ts", + "src/main/ipc/runtime-watcher-process-pool.test.ts", + "src/main/ipc/parcel-watcher-process-entry.test.ts", + "src/main/ipc/parcel-watcher-event-delivery.test.ts", + "src/main/ipc/filesystem-watcher-local-unsubscribe.test.ts", + "src/main/ipc/filesystem-watcher-wsl.test.ts", + "src/main/runtime/file-watcher-host.test.ts", + "src/main/runtime/rpc/methods/files.test.ts", + "src/main/providers/ssh-filesystem-provider.test.ts", + "src/renderer/src/runtime/runtime-file-client.test.ts", + "config/scripts/runtime-file-watcher-fault-harness.mjs" + ], + "assertionRefs": [ + { + "file": "src/main/ipc/parcel-watcher-process.test.ts", + "assertions": [ + "initial-subscribe crashes complete with an interruption refresh", + "IPC disconnect recovery restores every existing root before accepting new setup", + "queued abort removes one request without restarting healthy roots, while active abort or crawl-scoped timeout restores the shard", + "a crash-resubscribe crawl deadline ends only the stuck root and tears down its shard process", + "missing built entries fail closed and unavailable canary storage remains optional" + ] + }, + { + "file": "src/main/ipc/runtime-watcher-process-pool.test.ts", + "assertions": [ + "healthy roots spread across at most four shared supervisors before sharing", + "a supervisor-scoped failure spreads affected roots across at most four quarantine supervisors", + "a second fused quarantine creates no further replacement for that watch lifetime", + "a root-specific failure preserves healthy shard assignments", + "a live root whose crash-resubscribe times out moves to quarantine", + "an older same-root teardown cannot erase a newer watch lease's assignment" + ] + }, + { + "file": "src/main/ipc/parcel-watcher-process-entry.test.ts", + "assertions": [ + "queued cancellation removes the request before Parcel starts its crawl", + "active cancellation requests one host restart and emits no stale subscribed acknowledgement", + "runtime batches receive directory metadata before child IPC", + "an FSEvents dropped-events error is reported as recoverable and later events continue", + "batches above the configured event limit become one overflow before stat or IPC fanout", + "while process.send is backpressured, later bursts retain at most one bounded pending batch and collapse excess paths to overflow", + "unsubscribe discards pending delivery before it can perform stale stat work" + ] + }, + { + "file": "src/main/ipc/parcel-watcher-event-delivery.test.ts", + "assertions": [ + "eight concurrent root batches never exceed eight active directory metadata stats within one child" + ] + }, + { + "file": "src/main/ipc/filesystem-watcher-local-unsubscribe.test.ts", + "assertions": [ + "worktree deletion physically aborts pending native setup and settles before its crawl resolves", + "app shutdown physically aborts pending native setup and releases a late native handle" + ] + }, + { + "file": "src/main/runtime/file-watcher-host.test.ts", + "assertions": [ + "same-root clients share one physical watch and one client abort cannot cancel another owner's setup", + "shard failure preserves existing subscribers through isolated recovery and rejects stale-generation events", + "only failed isolated recovery ends subscribers, after a conservative overflow" + ] + }, + { + "file": "src/main/runtime/rpc/methods/files.test.ts", + "assertions": [ + "pre-ready transport abort reaches RuntimeFileCommands as the owning AbortSignal", + "terminal watcher loss flushes a final overflow change before error/end and unregisters cleanup" + ] + }, + { + "file": "src/main/providers/ssh-filesystem-provider.test.ts", + "assertions": [ + "an SSH caller abort rejects promptly without cancelling another same-root subscriber's shared setup", + "the physical relay request is cancelled when the last pending subscriber leaves", + "provider disposal cancels pending setup and sends one remote unwatch" + ] + }, + { + "file": "src/renderer/src/runtime/runtime-file-client.test.ts", + "assertions": [ + "terminal error evicts the shared renderer watch before an error callback can synchronously retry", + "the old stream's later end cannot evict or orphan the replacement stream", + "the end response releases retained preload and main-process subscription bookkeeping", + "a later same-root listener opens a fresh server subscription instead of reusing a dead stream" + ] + }, + { + "file": "config/scripts/runtime-file-watcher-fault-harness.mjs", + "assertions": [ + "the built watcher entry delivers an event before fault injection", + "the harness parent survives watcher-child SIGSEGV", + "the real supervisor automatically replaces the child, resubscribes, and delivers a post-crash event" + ] + } + ], + "evidenceRuns": [ + { + "date": "2026-07-12", + "runner": "local", + "platform": "macos", + "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/ipc/parcel-watcher-process.test.ts src/main/ipc/runtime-watcher-process-pool.test.ts src/main/ipc/parcel-watcher-process-entry.test.ts src/main/ipc/parcel-watcher-event-delivery.test.ts src/main/ipc/filesystem-watcher-local-unsubscribe.test.ts src/main/ipc/filesystem-watcher-wsl.test.ts src/main/runtime/file-watcher-host.test.ts src/main/runtime/rpc/methods/files.test.ts src/main/providers/ssh-filesystem-provider.test.ts src/renderer/src/runtime/runtime-file-client.test.ts", + "result": "passed", + "durationSeconds": 1.73, + "summary": "10 focused supervisor, pool, child-entry, delivery, desktop/WSL cleanup, runtime-host, RPC, SSH ownership, and renderer-client test files passed with 222 tests." + }, + { + "date": "2026-07-12", + "runner": "local", + "platform": "macos", + "command": "node config/scripts/runtime-file-watcher-fault-harness.mjs", + "result": "passed", + "durationSeconds": 5, + "summary": "All 26 built-entry SIGSEGV injections preserved the parent, replaced and resubscribed the child, and delivered a post-crash event; watcher-canary temp-directory count remained 6 before and after the final five-run leak check." + }, + { + "date": "2026-07-11", + "runner": "local", + "platform": "macos", + "command": "node config/scripts/runtime-file-watcher-fault-harness.mjs", + "result": "passed", + "durationSeconds": 0.3, + "summary": "The source-bundled real supervisor delivered through the built child before and after SIGSEGV; the parent survived and automatic resubscription used a replacement PID." + }, + { + "date": "2026-07-11", + "runner": "local", + "platform": "macos", + "command": "node config/scripts/runtime-file-watcher-fault-harness.mjs", + "result": "passed", + "durationSeconds": 2.2, + "summary": "Ten consecutive invocations survived, automatically replaced and resubscribed the child, and delivered a post-crash event. The watcher-canary temp-directory count remained 8 before and after the loop." + } + ], + "runtimeBudget": { + "p95Seconds": 60, + "scope": "focused process/RPC contract tests plus built child fault harness" + }, + "flakeHistory": { + "status": "unknown", + "evidence": "New deterministic unit coverage and ten consecutive local built-entry harness passes; still below the CI and soak history required for promotion." + }, + "redGreenEvidence": { + "status": "partial", + "evidence": "Red tests reproduced the reviewed failures: IPC disconnect-before-exit orphaned existing roots, queue-admission deadlines cascaded shard replacement, a fused shard created one child per root, destructive desktop cleanup waited indefinitely for native setup, and synchronous renderer retry joined a terminal stream. Earlier red coverage also reproduced missing-entry fail-open, terminal recovery loss, stale same-root lease deletion, and unbounded per-child stat lanes. The bounded quarantine and cancellation rewrite passes those contracts, and the real-supervisor built-entry harness survives SIGSEGV with automatic replacement. CI/soak history is still absent." + }, + "performanceBudget": { + "required": true, + "evidence": "Healthy operation is capped at four shared child processes instead of one child per root; based on the prior 51.9 MiB one-child measurement this bounds the unremeasured eight-root healthy ceiling near 207.6 MiB rather than the rejected 388.7 MiB per-root design. Degraded operation adds at most four quarantine children, distributes affected roots by least load, disposes empty slots, and permits no replacement after a quarantine shard itself fuses. Each child caps directory stats at eight across its assigned roots (at most 32 healthy or 64 total logical stat operations), each subscription retains at most one active plus one bounded pending batch, and no polling or serve-process stat fanout is added." + }, + "promotionCriteria": [ + "Run the built-entry SIGSEGV harness in soak for at least 100 consecutive passes or 14 days on macOS.", + "Add an eight-root/six-writer event-loop-delay soak artifact with an explicit latency budget.", + "Add a packaged orca serve pairing scenario that proves a connected client refreshes after child replacement." + ], + "knownGaps": [ + "The fault harness bundles the real source supervisor and drives the built child, but it does not launch a packaged orca serve pairing session or exercise app.asar path resolution.", + "The four-child healthy and four-child quarantine memory ceilings are inferred from the earlier one-child measurement; direct degraded-state RSS plus sustained six-writer event-loop-delay and CPU soak evidence is still absent.", + "Linux native watcher fault containment is source-equivalent but not live fault-injected in this evidence run.", + "Windows runtime watches bypass this path; SSH registration ownership is deterministic-contract tested but not live-relay fault-injected." + ], + "demotionRule": "Demote or quarantine if the fault harness flakes without a product or harness bug, if healthy operation exceeds four runtime watcher children, if total healthy plus quarantine operation exceeds eight, if quarantine children outlive their roots or repeat after fusing, if event delivery becomes unbounded, or if metadata/stat work returns to the serve process." } ] } diff --git a/config/scripts/package-electron-runtime-contract.test.mjs b/config/scripts/package-electron-runtime-contract.test.mjs index 5e70cf68f42..241cad088d1 100644 --- a/config/scripts/package-electron-runtime-contract.test.mjs +++ b/config/scripts/package-electron-runtime-contract.test.mjs @@ -91,6 +91,37 @@ describe('Electron runtime package contract', () => { ) }) + it('blocks Linux and macOS release packaging on watcher process fault recovery', () => { + const releaseWorkflow = parse( + readFileSync(join(projectDir, '.github/workflows/release-cut.yml'), 'utf8') + ) + const macWorkflow = parse( + readFileSync(join(projectDir, '.github/workflows/release-mac-build.yml'), 'utf8') + ) + const assertFaultGate = (steps, publishStepName, expectedCondition) => { + const names = steps.map((step) => step.name) + const gate = steps.find((step) => step.name === 'Gate runtime file-watcher process isolation') + + expect(gate.if).toBe(expectedCondition) + expect(gate['continue-on-error']).toBeUndefined() + expect(gate.run).toContain('node config/scripts/runtime-file-watcher-fault-harness.mjs') + expect(gate.run).toContain('ELECTRON_RUN_AS_NODE=1 pnpm exec electron') + expect(names.indexOf('Build app')).toBeLessThan(names.indexOf(gate.name)) + expect(names.indexOf(gate.name)).toBeLessThan(names.indexOf(publishStepName)) + } + + assertFaultGate( + releaseWorkflow.jobs.build.steps, + 'Publish release artifacts (Linux)', + "runner.os == 'Linux'" + ) + assertFaultGate( + macWorkflow.jobs['build-mac'].steps, + 'Publish release artifacts (macOS)', + undefined + ) + }) + it('pins the Windows release builder to the VS 2022 runner image', () => { const releaseWorkflow = parse( readFileSync(join(projectDir, '.github/workflows/release-cut.yml'), 'utf8') diff --git a/config/scripts/runtime-file-watcher-fault-harness.mjs b/config/scripts/runtime-file-watcher-fault-harness.mjs new file mode 100644 index 00000000000..88fec4245fb --- /dev/null +++ b/config/scripts/runtime-file-watcher-fault-harness.mjs @@ -0,0 +1,180 @@ +import { createRequire } from 'node:module' +import { existsSync } from 'node:fs' +import { mkdtemp, realpath, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' +import { build } from 'esbuild' + +const ENTRY_PATH = resolve('out/main/parcel-watcher-process-entry.js') +const SUPERVISOR_SOURCE = resolve('src/main/ipc/parcel-watcher-process-supervisor.ts') +const WAIT_TIMEOUT_MS = 15_000 +const require = createRequire(import.meta.url) + +function withTimeout(promise, label) { + return new Promise((resolvePromise, rejectPromise) => { + const timer = setTimeout( + () => rejectPromise(new Error(`Timed out waiting for ${label}`)), + WAIT_TIMEOUT_MS + ) + promise.then( + (value) => { + clearTimeout(timer) + resolvePromise(value) + }, + (error) => { + clearTimeout(timer) + rejectPromise(error) + } + ) + }) +} + +function nextMatchingEvent(register, predicate, label) { + return withTimeout( + new Promise((resolveEvent) => { + register((events) => { + if (events.some(predicate)) { + resolveEvent(events) + } + }) + }), + label + ) +} + +async function loadSupervisor(bundleDir) { + const outfile = join(bundleDir, 'watcher-supervisor.cjs') + await build({ + entryPoints: [SUPERVISOR_SOURCE], + bundle: true, + platform: 'node', + format: 'cjs', + outfile, + external: ['@parcel/watcher', 'electron'], + logLevel: 'silent' + }) + return require(outfile).WatcherProcessSupervisor +} + +async function main() { + if (process.platform === 'win32') { + console.log('[runtime-file-watcher-fault] SKIP: SIGSEGV oracle is macOS/Linux only') + return + } + if (!existsSync(ENTRY_PATH)) { + throw new Error(`Missing ${ENTRY_PATH}; run pnpm run build:electron-vite first`) + } + + // Why: mkdtemp/realpath/bundle/construction can fail before the body runs. + // Keep cleanup in finally from the first successful mkdtemp onward, and clean + // the original temp path if realpath never succeeds. + let createdRootPath + let bundleDir + let rootPath + let supervisor + let subscription + let watcherCanaryDir + let eventListener = () => undefined + let rejectWatcherError + const watcherError = new Promise((_, reject) => { + rejectWatcherError = reject + }) + // Attach early so a callback rejection before the race cannot become unhandled. + watcherError.catch(() => undefined) + + try { + createdRootPath = await mkdtemp(join(tmpdir(), 'orca-runtime-watcher-fault-')) + bundleDir = await mkdtemp(join(tmpdir(), 'orca-runtime-watcher-harness-')) + // Parcel reports canonical event paths on macOS, where tmpdir() may use the + // /var symlink spelling. Keep the oracle in the same path domain. + rootPath = await realpath(createdRootPath) + const WatcherProcessSupervisor = await loadSupervisor(bundleDir) + supervisor = new WatcherProcessSupervisor() + + let resolveInterruption + const interrupted = withTimeout( + new Promise((resolveWait) => { + resolveInterruption = resolveWait + }), + 'automatic watcher resubscription' + ) + subscription = await supervisor.subscribe( + rootPath, + (error, events) => { + if (error) { + // Why: throws from the async watcher callback escape main()'s try/finally + // and skip teardown. Surface failures through a harness promise instead. + rejectWatcherError(error) + return + } + eventListener(events) + }, + { ignore: ['.git', 'node_modules'] }, + { + delivery: { includeDirectoryMetadata: true, maxEventsPerBatch: 200 }, + onInterruption: () => resolveInterruption() + } + ) + watcherCanaryDir = supervisor.canaryDir + + const beforeEvent = nextMatchingEvent( + (listener) => { + eventListener = listener + }, + (event) => event.path === join(rootPath, 'before.txt'), + 'pre-crash watch event' + ) + await writeFile(join(rootPath, 'before.txt'), 'before') + await Promise.race([beforeEvent, watcherError]) + + const firstChildPid = supervisor.child?.pid + if (!firstChildPid) { + throw new Error('Watcher supervisor did not expose a live child') + } + process.kill(firstChildPid, 'SIGSEGV') + await Promise.race([interrupted, watcherError]) + + const replacementChildPid = supervisor.child?.pid + if (!replacementChildPid || replacementChildPid === firstChildPid) { + throw new Error('Watcher supervisor did not replace the faulted child') + } + const afterEvent = nextMatchingEvent( + (listener) => { + eventListener = listener + }, + (event) => event.path === join(rootPath, 'after.txt'), + 'post-crash watch event' + ) + await writeFile(join(rootPath, 'after.txt'), 'after') + await Promise.race([afterEvent, watcherError]) + + console.log( + JSON.stringify({ + hostPid: process.pid, + killedWatcherPid: firstChildPid, + replacementWatcherPid: replacementChildPid, + hostSurvived: true, + automaticResubscribe: true, + postCrashEventDelivered: true + }) + ) + } finally { + try { + await subscription?.unsubscribe() + } finally { + supervisor?.dispose() + await Promise.all([ + createdRootPath ? rm(createdRootPath, { recursive: true, force: true }) : Promise.resolve(), + rootPath && rootPath !== createdRootPath + ? rm(rootPath, { recursive: true, force: true }) + : Promise.resolve(), + bundleDir ? rm(bundleDir, { recursive: true, force: true }) : Promise.resolve() + ]) + } + } + if (watcherCanaryDir && existsSync(watcherCanaryDir)) { + throw new Error(`Watcher supervisor leaked its canary directory: ${watcherCanaryDir}`) + } +} + +await main() diff --git a/electron.vite.config.ts b/electron.vite.config.ts index dbe2fbc02b3..7b8c9cb8318 100644 --- a/electron.vite.config.ts +++ b/electron.vite.config.ts @@ -182,7 +182,6 @@ export default defineConfig({ 'computer-sidecar': resolve('src/main/computer/sidecar-entry.ts'), 'stt-worker': resolve('src/main/speech/stt-worker.ts'), 'warp-theme-parser-worker': resolve('src/main/warp-themes/warp-theme-parser-worker.ts'), - 'file-watcher-worker': resolve('src/main/runtime/file-watcher-worker.ts'), // Why: forked with ELECTRON_RUN_AS_NODE so @parcel/watcher faults // can't take down the main process (issue #7547). 'parcel-watcher-process-entry': resolve('src/main/ipc/parcel-watcher-process-entry.ts'), diff --git a/src/main/ipc/filesystem-watcher-local-unsubscribe.test.ts b/src/main/ipc/filesystem-watcher-local-unsubscribe.test.ts index d0891ec35a2..a566bf178c6 100644 --- a/src/main/ipc/filesystem-watcher-local-unsubscribe.test.ts +++ b/src/main/ipc/filesystem-watcher-local-unsubscribe.test.ts @@ -300,6 +300,48 @@ describe('local filesystem watcher unsubscribe cleanup', () => { } }) + it('aborts an opening local watcher when the last listener unwatches', async () => { + vi.mocked(stat).mockResolvedValue({ isDirectory: () => true } as never) + let resolveSubscribe: (subscription: { unsubscribe: () => void }) => void = () => {} + const unsubscribeMock = vi.fn() + vi.mocked(subscribeParcelWatcher).mockImplementation( + () => + new Promise((resolve) => { + resolveSubscribe = resolve as typeof resolveSubscribe + }) + ) + const sender = { + isDestroyed: () => false, + send: vi.fn(), + once: vi.fn(), + id: 1 + } + + const watchPromise = handlers['fs:watchWorktree']( + { sender }, + { worktreePath: '/tmp/repo' } + ) as Promise + await vi.waitFor(() => { + expect(subscribeParcelWatcher).toHaveBeenCalled() + }) + + const unwatchPromise = handlers['fs:unwatchWorktree']( + { sender }, + { worktreePath: '/tmp/repo' } + ) as Promise + // Why: unwatch must abort the in-flight install so watchWorktree settles + // without waiting for the native subscribe crawl to finish. + const watchSettledEarly = await Promise.race([ + watchPromise.then(() => true), + new Promise((resolve) => setTimeout(() => resolve(false), 20)) + ]) + resolveSubscribe({ unsubscribe: unsubscribeMock }) + await Promise.all([watchPromise, unwatchPromise]) + + expect(watchSettledEarly).toBe(true) + expect(unsubscribeMock).toHaveBeenCalledTimes(1) + }) + it('cancels an opening local watcher for worktree deletion', async () => { vi.mocked(stat).mockResolvedValue({ isDirectory: () => true } as never) let resolveSubscribe: (subscription: { unsubscribe: () => void }) => void = () => {} @@ -325,9 +367,50 @@ describe('local filesystem watcher unsubscribe cleanup', () => { expect(subscribeParcelWatcher).toHaveBeenCalled() }) const closePromise = closeLocalWatcherForWorktreePath('/tmp/repo') + const closedBeforeNativeSubscribe = await Promise.race([ + closePromise.then(() => true), + new Promise((resolve) => setTimeout(() => resolve(false), 0)) + ]) resolveSubscribe({ unsubscribe: unsubscribeMock }) await Promise.all([watchPromise, closePromise]) + expect(closedBeforeNativeSubscribe).toBe(true) + expect(unsubscribeMock).toHaveBeenCalledTimes(1) + }) + + it('cancels an opening local watcher during app shutdown', async () => { + vi.mocked(stat).mockResolvedValue({ isDirectory: () => true } as never) + let resolveSubscribe: (subscription: { unsubscribe: () => void }) => void = () => {} + const unsubscribeMock = vi.fn() + vi.mocked(subscribeParcelWatcher).mockImplementation( + () => + new Promise((resolve) => { + resolveSubscribe = resolve as typeof resolveSubscribe + }) + ) + const sender = { + isDestroyed: () => false, + send: vi.fn(), + once: vi.fn(), + id: 1 + } + + const watchPromise = handlers['fs:watchWorktree']( + { sender }, + { worktreePath: '/tmp/repo' } + ) as Promise + await vi.waitFor(() => { + expect(subscribeParcelWatcher).toHaveBeenCalled() + }) + const shutdownPromise = closeAllWatchers() + const closedBeforeNativeSubscribe = await Promise.race([ + shutdownPromise.then(() => true), + new Promise((resolve) => setTimeout(() => resolve(false), 0)) + ]) + resolveSubscribe({ unsubscribe: unsubscribeMock }) + await Promise.all([watchPromise, shutdownPromise]) + + expect(closedBeforeNativeSubscribe).toBe(true) expect(unsubscribeMock).toHaveBeenCalledTimes(1) }) }) diff --git a/src/main/ipc/filesystem-watcher-wsl.test.ts b/src/main/ipc/filesystem-watcher-wsl.test.ts index 203ca2fbfa1..97b21dddda3 100644 --- a/src/main/ipc/filesystem-watcher-wsl.test.ts +++ b/src/main/ipc/filesystem-watcher-wsl.test.ts @@ -172,4 +172,26 @@ describe('createWslWatcher', () => { await expect(promise).rejects.toThrow('write EPIPE') }) + + it('kills the snapshot process when the install abort signal fires during startup', async () => { + const child = new FakeChildProcess() + spawnMock.mockReturnValueOnce(child) + const controller = new AbortController() + const promise = createWslWatcher(ROOT_KEY, ROOT_KEY, makeDeps(), controller.signal) + + controller.abort() + + await expect(promise).rejects.toMatchObject({ name: 'AbortError' }) + expect(child.kill).toHaveBeenCalledOnce() + }) + + it('rejects immediately when the install abort signal is already aborted', async () => { + const controller = new AbortController() + controller.abort() + + await expect(createWslWatcher(ROOT_KEY, ROOT_KEY, makeDeps(), controller.signal)).rejects.toMatchObject({ + name: 'AbortError' + }) + expect(spawnMock).not.toHaveBeenCalled() + }) }) diff --git a/src/main/ipc/filesystem-watcher-wsl.ts b/src/main/ipc/filesystem-watcher-wsl.ts index b0d2c8e9d04..54e58415894 100644 --- a/src/main/ipc/filesystem-watcher-wsl.ts +++ b/src/main/ipc/filesystem-watcher-wsl.ts @@ -150,8 +150,15 @@ function markOverflowWithoutUncStat(root: WatchedRoot): void { export async function createWslWatcher( rootKey: string, worktreePath: string, - deps: WslWatcherDeps + deps: WslWatcherDeps, + signal?: AbortSignal ): Promise { + // Why: cancelled local installs pass the same abort controller used for + // native Parcel subscribe; honor it before spawning a WSL snapshot process. + if (signal?.aborted) { + throw new DOMException('WSL watcher subscription aborted', 'AbortError') + } + const wsl = parseWslUncPath(worktreePath) if (!wsl) { throw new Error(`Not a WSL path: ${worktreePath}`) @@ -257,6 +264,12 @@ export async function createWslWatcher( throw error instanceof Error ? error : new Error(String(error)) } + const onAbort = (): void => { + settleInitial(new DOMException('WSL watcher subscription aborted', 'AbortError')) + child.kill() + } + signal?.addEventListener('abort', onAbort, { once: true }) + const startupTimer = setTimeout(() => { settleInitial(new Error(`Timed out starting WSL watcher for ${worktreePath}`)) child.kill() @@ -321,7 +334,12 @@ export async function createWslWatcher( child.stdin.end(buildSnapshotScript(deps.ignoreDirs)) - await initialSnapshotReady.finally(() => clearTimeout(startupTimer)) + try { + await initialSnapshotReady + } finally { + clearTimeout(startupTimer) + signal?.removeEventListener('abort', onAbort) + } root.subscription = { unsubscribe: async () => { diff --git a/src/main/ipc/filesystem-watcher.ts b/src/main/ipc/filesystem-watcher.ts index 224c348b871..983fc9ce985 100644 --- a/src/main/ipc/filesystem-watcher.ts +++ b/src/main/ipc/filesystem-watcher.ts @@ -72,6 +72,7 @@ const pendingLocalUnsubscribes = new Set>() type LocalWatcherInstallToken = { cancelled: boolean listeners: Map + abortController: AbortController } type LocalWatcherInstallResult = 'installed' | 'unavailable' | 'cancelled' // Why: native watcher creation is async. Concurrent local watch requests for @@ -96,6 +97,9 @@ function cleanupInFlightLocalInstallsForSender(senderId: number): void { token.listeners.delete(senderId) if (token.listeners.size === 0) { token.cancelled = true + // Why: match closeLocalWatcherForWorktreePath / closeAllWatchers — abort + // so a pending native/forked subscription stops early, not at completion. + token.abortController.abort() } } } @@ -278,7 +282,11 @@ function scheduleBatchFlush(rootKey: string, root: WatchedRoot): void { // ── Watcher creation ───────────────────────────────────────────────── -async function createWatcher(rootKey: string, rootPath: string): Promise { +async function createWatcher( + rootKey: string, + rootPath: string, + signal?: AbortSignal +): Promise { const root: WatchedRoot = { subscription: null!, listeners: new Map(), @@ -300,6 +308,11 @@ async function createWatcher(rootKey: string, rootPath: string): Promise { + root.batch.overflowed = true + scheduleBatchFlush(rootKey, root) + } + // Why: subscriptions run in a forked watcher process (issue #7547 — // watcher.node teardown races fail-fast the hosting process). A watcher // crash there is recovered by resubscribing; onInterruption marks the @@ -336,11 +349,13 @@ async function createWatcher(rootKey: string, rootPath: string): Promise { - // The watcher process crashed and this root was resubscribed; events - // during the gap are lost, so hand the renderer an overflow refresh. - root.batch.overflowed = true - scheduleBatchFlush(rootKey, root) + { + delivery: { maxEventsPerBatch: MAX_BATCHED_WATCHER_EVENTS }, + // A child restart or bounded-queue overflow loses path precision; both + // require the same conservative renderer refresh. + onInterruption: markWatcherInterrupted, + onOverflow: markWatcherInterrupted, + signal } ) @@ -466,7 +481,11 @@ async function subscribe(worktreePath: string, sender: WebContents): Promise undefined) @@ -926,6 +959,7 @@ export async function closeAllWatchers(): Promise { for (const token of inFlightLocalInstalls.values()) { token.listeners.clear() token.cancelled = true + token.abortController.abort() } for (const [rootKey, root] of watchedRoots) { diff --git a/src/main/ipc/parcel-watcher-canary-directory.ts b/src/main/ipc/parcel-watcher-canary-directory.ts new file mode 100644 index 00000000000..28e7ec6518c --- /dev/null +++ b/src/main/ipc/parcel-watcher-canary-directory.ts @@ -0,0 +1,26 @@ +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +export function createWatcherCanaryDirectory(): string | null { + try { + return mkdtempSync(join(tmpdir(), 'orca-watcher-canary-')) + } catch (error) { + // Why: the canary is diagnostic only; read-only temp storage must not + // disable the crash-isolated watcher itself. + console.warn('[parcel-watcher-process] canary directory unavailable:', error) + return null + } +} + +export function removeWatcherCanaryDirectory(canaryDir: string | null): null { + if (!canaryDir) { + return null + } + try { + rmSync(canaryDir, { recursive: true, force: true }) + } catch { + // Canary cleanup is best-effort; watcher lifecycle must still complete. + } + return null +} diff --git a/src/main/ipc/parcel-watcher-child-launch.ts b/src/main/ipc/parcel-watcher-child-launch.ts new file mode 100644 index 00000000000..cf8e7348bc0 --- /dev/null +++ b/src/main/ipc/parcel-watcher-child-launch.ts @@ -0,0 +1,46 @@ +import { fork, type ChildProcess } from 'node:child_process' +import { + createWatcherCanaryDirectory, + removeWatcherCanaryDirectory +} from './parcel-watcher-canary-directory' +import type { WatcherToHostMessage } from './parcel-watcher-process-protocol' + +export type LaunchedWatcherChild = { + child: ChildProcess + canaryDir: string | null +} + +export function launchWatcherChild( + entryPath: string, + currentCanaryDir: string | null, + onMessage: (child: ChildProcess, message: WatcherToHostMessage) => void, + onGone: (child: ChildProcess, code?: number | null, signal?: NodeJS.Signals | null) => void +): LaunchedWatcherChild | null { + const canaryDir = currentCanaryDir ?? createWatcherCanaryDirectory() + let child: ChildProcess + try { + child = fork(entryPath, [], { + stdio: ['ignore', 'ignore', 'pipe', 'ipc'], + env: { + ...process.env, + ELECTRON_RUN_AS_NODE: '1', + ...(canaryDir ? { ORCA_WATCHER_CANARY_DIR: canaryDir } : {}) + }, + ...(process.platform === 'win32' ? { windowsHide: true } : {}) + }) + } catch (error) { + removeWatcherCanaryDirectory(canaryDir) + console.error('[parcel-watcher-process] failed to fork watcher process:', error) + return null + } + child.stderr?.on('data', (chunk: Buffer) => { + console.error('[parcel-watcher-process]', String(chunk).trimEnd()) + }) + child.on('message', (message) => onMessage(child, message as WatcherToHostMessage)) + // Why: Node can close IPC before emitting exit. Recover at disconnect so a + // concurrent subscribe cannot replace the child without restoring old roots. + child.on('disconnect', () => onGone(child)) + child.on('error', () => onGone(child)) + child.on('exit', (code, signal) => onGone(child, code, signal)) + return { child, canaryDir } +} diff --git a/src/main/ipc/parcel-watcher-crash-fuse.ts b/src/main/ipc/parcel-watcher-crash-fuse.ts new file mode 100644 index 00000000000..69717311ba5 --- /dev/null +++ b/src/main/ipc/parcel-watcher-crash-fuse.ts @@ -0,0 +1,24 @@ +const CRASH_WINDOW_MS = 30_000 +const MAX_CRASHES_PER_WINDOW = 3 + +export class WatcherProcessCrashFuse { + private crashTimes: number[] = [] + + recordCrash(now = Date.now()): void { + this.removeExpired(now) + this.crashTimes.push(now) + } + + isOpen(now = Date.now()): boolean { + this.removeExpired(now) + return this.crashTimes.length >= MAX_CRASHES_PER_WINDOW + } + + reset(): void { + this.crashTimes = [] + } + + private removeExpired(now: number): void { + this.crashTimes = this.crashTimes.filter((time) => now - time < CRASH_WINDOW_MS) + } +} diff --git a/src/main/ipc/parcel-watcher-event-delivery.test.ts b/src/main/ipc/parcel-watcher-event-delivery.test.ts new file mode 100644 index 00000000000..66c6232a63f --- /dev/null +++ b/src/main/ipc/parcel-watcher-event-delivery.test.ts @@ -0,0 +1,44 @@ +import { setImmediate } from 'node:timers/promises' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { statMock } = vi.hoisted(() => ({ statMock: vi.fn() })) + +vi.mock('node:fs/promises', () => ({ stat: statMock })) + +import { prepareWatcherProcessEvents } from './parcel-watcher-event-delivery' + +describe('prepareWatcherProcessEvents', () => { + beforeEach(() => { + statMock.mockReset() + }) + + it('caps directory metadata stat work across concurrent roots in one child', async () => { + let activeStats = 0 + let maxActiveStats = 0 + statMock.mockImplementation(async () => { + activeStats++ + maxActiveStats = Math.max(maxActiveStats, activeStats) + await setImmediate() + activeStats-- + return { isDirectory: () => false } + }) + const batches = Array.from({ length: 8 }, (_, rootIndex) => + Array.from({ length: 16 }, (_, eventIndex) => ({ + type: 'update' as const, + path: `/repo-${rootIndex}/file-${eventIndex}.ts` + })) + ) + + await Promise.all( + batches.map((events) => + prepareWatcherProcessEvents(events, { + includeDirectoryMetadata: true, + maxEventsPerBatch: 200 + }) + ) + ) + + expect(statMock).toHaveBeenCalledTimes(128) + expect(maxActiveStats).toBe(8) + }) +}) diff --git a/src/main/ipc/parcel-watcher-event-delivery.ts b/src/main/ipc/parcel-watcher-event-delivery.ts new file mode 100644 index 00000000000..54e8448801c --- /dev/null +++ b/src/main/ipc/parcel-watcher-event-delivery.ts @@ -0,0 +1,160 @@ +import { stat } from 'node:fs/promises' +import type { Event as ParcelWatcherEvent } from '@parcel/watcher' +import { MAX_BATCHED_WATCHER_EVENTS } from './filesystem-watcher-event-batch' +import type { + WatcherProcessDeliveryOptions, + WatcherProcessEvent +} from './parcel-watcher-process-protocol' + +const DIRECTORY_STAT_CONCURRENCY = 8 +let activeDirectoryStats = 0 +const directoryStatWaiters: (() => void)[] = [] + +export type WatcherProcessEventDeliveryQueue = { + enqueue(events: readonly ParcelWatcherEvent[]): void + close(): void +} + +async function acquireDirectoryStatSlot(): Promise { + if (activeDirectoryStats < DIRECTORY_STAT_CONCURRENCY) { + activeDirectoryStats++ + return + } + await new Promise((resolve) => directoryStatWaiters.push(resolve)) +} + +function releaseDirectoryStatSlot(): void { + const next = directoryStatWaiters.shift() + if (next) { + // Transfer the existing slot directly so a newly arriving task cannot + // overtake this waiter and temporarily exceed the global budget. + next() + return + } + activeDirectoryStats-- +} + +async function statWatcherEventPath(eventPath: string): Promise { + await acquireDirectoryStatSlot() + try { + return (await stat(eventPath)).isDirectory() + } finally { + releaseDirectoryStatSlot() + } +} + +async function mapWithConcurrency( + items: readonly T[], + limit: number, + mapper: (item: T) => Promise +): Promise { + const results = Array.from({ length: items.length }) + let cursor = 0 + const lane = async (): Promise => { + while (cursor < items.length) { + const index = cursor++ + results[index] = await mapper(items[index]) + } + } + await Promise.all(Array.from({ length: Math.min(limit, items.length) }, lane)) + return results +} + +async function mapWatcherEvent( + event: ParcelWatcherEvent, + includeDirectoryMetadata: boolean +): Promise { + if (!includeDirectoryMetadata || event.type === 'delete') { + return { type: event.type, path: event.path } + } + let isDirectory = false + try { + isDirectory = await statWatcherEventPath(event.path) + } catch { + // Why: a path can vanish between the native event and metadata lookup. + // Treat unknown metadata as a file-like event so parent invalidation still runs. + } + return { type: event.type, path: event.path, isDirectory } +} + +export async function prepareWatcherProcessEvents( + events: readonly ParcelWatcherEvent[], + delivery: WatcherProcessDeliveryOptions | undefined +): Promise { + if (delivery?.maxEventsPerBatch !== undefined && events.length > delivery.maxEventsPerBatch) { + return null + } + if (delivery?.includeDirectoryMetadata !== true) { + return events.map((event) => ({ type: event.type, path: event.path })) + } + return mapWithConcurrency(events, DIRECTORY_STAT_CONCURRENCY, (event) => + mapWatcherEvent(event, true) + ) +} + +/** Keep at most one active and one bounded pending batch per subscription. */ +export function createWatcherProcessEventDeliveryQueue( + delivery: WatcherProcessDeliveryOptions | undefined, + deliver: (events: WatcherProcessEvent[] | null) => Promise, + onError: (error: unknown) => void +): WatcherProcessEventDeliveryQueue { + const eventLimit = delivery?.maxEventsPerBatch ?? MAX_BATCHED_WATCHER_EVENTS + let active = true + let draining = false + let pendingOverflow = false + let pendingEvents: ParcelWatcherEvent[] = [] + + const drain = async (): Promise => { + if (!active || draining) { + return + } + draining = true + try { + while (active && (pendingOverflow || pendingEvents.length > 0)) { + const overflowed = pendingOverflow + const events = pendingEvents + pendingOverflow = false + pendingEvents = [] + if (overflowed) { + await deliver(null) + continue + } + const prepared = await prepareWatcherProcessEvents(events, delivery) + if (active) { + await deliver(prepared) + } + } + } catch (error) { + if (active) { + onError(error) + } + } finally { + draining = false + if (active && (pendingOverflow || pendingEvents.length > 0)) { + void drain() + } + } + } + + return { + enqueue(events): void { + if (!active || events.length === 0 || pendingOverflow) { + return + } + if (events.length > eventLimit || pendingEvents.length + events.length > eventLimit) { + pendingEvents = [] + pendingOverflow = true + } else { + for (const event of events) { + pendingEvents.push(event) + } + } + void drain() + }, + close(): void { + active = false + pendingEvents = [] + pendingOverflow = false + } + } +} diff --git a/src/main/ipc/parcel-watcher-host-subscriptions.ts b/src/main/ipc/parcel-watcher-host-subscriptions.ts new file mode 100644 index 00000000000..fd6b9a0257a --- /dev/null +++ b/src/main/ipc/parcel-watcher-host-subscriptions.ts @@ -0,0 +1,140 @@ +import type { ChildProcess } from 'node:child_process' +import { + resetPendingSubscribeAttempt, + takePendingSubscribe +} from './parcel-watcher-pending-subscribe' +import { WatcherProcessFailure } from './parcel-watcher-process-failure' +import type { HostToWatcherMessage, WatcherToHostMessage } from './parcel-watcher-process-protocol' +import type { + WatcherProcessSubscription, + WatcherProcessSubscriptionRecord +} from './parcel-watcher-process-subscription' + +export function handleWatcherHostMessage( + message: Exclude< + WatcherToHostMessage, + { op: 'subscribe-started' } | { op: 'cancel-requires-restart' } + >, + records: Map, + pendingUnsubscribes: Map void>, + reportTerminalError: (record: WatcherProcessSubscriptionRecord, error: Error) => void, + killWatcherChildIfIdle: () => void +): void { + if (message.op === 'unsubscribed') { + const resolve = pendingUnsubscribes.get(message.id) + pendingUnsubscribes.delete(message.id) + resolve?.() + return + } + const record = records.get(message.id) + if (!record) { + return + } + if (message.op === 'subscribed') { + resetPendingSubscribeAttempt(record) + takePendingSubscribe(record)?.resolve() + if (record.interrupted) { + record.interrupted = false + record.hooks.onInterruption?.() + } + return + } + if (message.op === 'subscribe-failed') { + records.delete(message.id) + resetPendingSubscribeAttempt(record) + const pending = takePendingSubscribe(record) + const error = new WatcherProcessFailure(message.message, 'subscription', 'subscribe_failed') + if (pending) { + pending.reject(error) + } else { + reportTerminalError(record, error) + } + killWatcherChildIfIdle() + return + } + if (message.op === 'events') { + record.callback(null, message.events) + return + } + if (message.op === 'overflow') { + record.hooks.onOverflow?.() + return + } + record.callback(new Error(message.message), []) +} + +export function reportWatcherTerminalError( + record: WatcherProcessSubscriptionRecord, + error: Error +): void { + if (record.hooks.onTerminalError) { + record.hooks.onTerminalError(error) + return + } + record.callback(error, []) +} + +export function failAllWatcherSubscriptions( + records: Map, + error: Error +): void { + // Snapshot first: onTerminalError hooks can dispose the supervisor and clear `records`. + for (const record of Array.from(records.values())) { + resetPendingSubscribeAttempt(record) + const pending = takePendingSubscribe(record) + if (pending) { + pending.reject(error) + } else { + reportWatcherTerminalError(record, error) + } + } + records.clear() +} + +export function resolvePendingWatcherUnsubscribes( + pendingUnsubscribes: Map void> +): void { + for (const resolve of pendingUnsubscribes.values()) { + resolve() + } + pendingUnsubscribes.clear() +} + +export type CreateHostWatcherSubscriptionOptions = { + record: WatcherProcessSubscriptionRecord + records: Map + pendingUnsubscribes: Map void> + getChild: () => ChildProcess | null + killWatcherChildIfIdle: () => void + sendToChild: (child: ChildProcess, message: HostToWatcherMessage) => void +} + +export function createHostWatcherSubscription({ + record, + records, + pendingUnsubscribes, + getChild, + killWatcherChildIfIdle, + sendToChild +}: CreateHostWatcherSubscriptionOptions): WatcherProcessSubscription { + return { + unsubscribe: (): Promise => { + if (!records.delete(record.id)) { + return Promise.resolve() + } + resetPendingSubscribeAttempt(record) + const child = getChild() + if (!child?.connected) { + return Promise.resolve() + } + if (records.size === 0) { + killWatcherChildIfIdle() + return Promise.resolve() + } + return new Promise((resolve) => { + pendingUnsubscribes.set(record.id, resolve) + sendToChild(child, { op: 'unsubscribe', id: record.id }) + }) + } + } +} diff --git a/src/main/ipc/parcel-watcher-in-process-fallback.ts b/src/main/ipc/parcel-watcher-in-process-fallback.ts new file mode 100644 index 00000000000..bcd2812d35d --- /dev/null +++ b/src/main/ipc/parcel-watcher-in-process-fallback.ts @@ -0,0 +1,124 @@ +import type * as ParcelWatcher from '@parcel/watcher' +import { createWatcherProcessEventDeliveryQueue } from './parcel-watcher-event-delivery' +import { WatcherProcessFailure } from './parcel-watcher-process-failure' +import type { + WatcherProcessCallback, + WatcherProcessHooks, + WatcherProcessSubscription +} from './parcel-watcher-process-subscription' +import type { WatcherProcessSubscribeOptions } from './parcel-watcher-process-protocol' + +/** + * Use Parcel directly when Vitest mocks it. Production builds fail closed if + * the crash-isolated child entry is unavailable. + */ +export async function subscribeWithInProcessWatcher( + dir: string, + callback: WatcherProcessCallback, + opts: WatcherProcessSubscribeOptions, + hooks: WatcherProcessHooks +): Promise { + if (hooks.signal?.aborted) { + throw new WatcherProcessFailure( + 'file watcher subscription aborted', + 'subscription', + 'subscribe_aborted' + ) + } + let abortListener: (() => void) | undefined + let timer: ReturnType | undefined + const cancellation = new Promise((_resolve, reject) => { + if (hooks.signal) { + abortListener = () => + reject( + new WatcherProcessFailure( + 'file watcher subscription aborted', + 'subscription', + 'subscribe_aborted' + ) + ) + hooks.signal.addEventListener('abort', abortListener, { once: true }) + } + if (hooks.subscribeTimeoutMs !== undefined) { + timer = setTimeout(() => { + reject( + new WatcherProcessFailure( + `file watcher subscription timed out after ${hooks.subscribeTimeoutMs}ms`, + 'subscription', + 'subscribe_timeout' + ) + ) + }, hooks.subscribeTimeoutMs) + timer.unref?.() + } + }) + const clearPendingControls = (): void => { + if (abortListener && hooks.signal) { + hooks.signal.removeEventListener('abort', abortListener) + } + if (timer) { + clearTimeout(timer) + } + } + let watcher: typeof ParcelWatcher + try { + // Why: setup ownership starts before module loading; an abort or timeout + // during the import must settle the caller just like one during the crawl. + watcher = await Promise.race([import('@parcel/watcher'), cancellation]) + } catch (error) { + clearPendingControls() + throw error + } + let active = true + const eventDelivery = createWatcherProcessEventDeliveryQueue( + hooks.delivery, + async (events) => { + if (!active) { + return + } + if (events === null) { + hooks.onOverflow?.() + } else { + callback(null, events) + } + }, + (error) => callback(error instanceof Error ? error : new Error(String(error)), []) + ) + const subscriptionPromise = watcher.subscribe( + dir, + (err, events) => { + if (!active) { + return + } + if (err) { + callback(err, []) + return + } + eventDelivery.enqueue(events) + }, + opts as ParcelWatcher.Options + ) + let subscription: ParcelWatcher.AsyncSubscription + try { + subscription = await Promise.race([subscriptionPromise, cancellation]) + } catch (error) { + active = false + eventDelivery.close() + clearPendingControls() + // Why: Parcel cannot cancel a crawl in-process. If it eventually finishes, + // release the late native handle instead of leaking it into the test host. + // Swallow unsubscribe failures so they never surface as unhandled rejections. + void subscriptionPromise + .then((lateSubscription) => lateSubscription.unsubscribe()) + .catch(() => undefined) + throw error + } + clearPendingControls() + return { + unsubscribe: async (): Promise => { + active = false + eventDelivery.close() + await subscription.unsubscribe() + } + } +} diff --git a/src/main/ipc/parcel-watcher-pending-subscribe.ts b/src/main/ipc/parcel-watcher-pending-subscribe.ts new file mode 100644 index 00000000000..67a020b9d5e --- /dev/null +++ b/src/main/ipc/parcel-watcher-pending-subscribe.ts @@ -0,0 +1,105 @@ +import { WatcherProcessFailure } from './parcel-watcher-process-failure' +import type { + PendingWatcherProcessSubscribe, + WatcherProcessSubscriptionRecord +} from './parcel-watcher-process-subscription' + +export function installPendingSubscribeControls( + record: WatcherProcessSubscriptionRecord, + cancel: (error: WatcherProcessFailure) => void +): void { + const pending = record.pendingSubscribe + if (!pending) { + return + } + if (record.hooks.signal) { + pending.abortListener = () => + cancel( + new WatcherProcessFailure( + 'file watcher subscription aborted', + 'subscription', + 'subscribe_aborted' + ) + ) + record.hooks.signal.addEventListener('abort', pending.abortListener, { once: true }) + } +} + +export function startPendingSubscribeTimeout( + record: WatcherProcessSubscriptionRecord, + cancel: (error: WatcherProcessFailure) => void +): void { + const pending = record.pendingSubscribe + if (!pending) { + return + } + record.crawlStarted = true + if (pending.timer || record.hooks.subscribeTimeoutMs === undefined) { + return + } + pending.timer = setTimeout(() => { + cancel( + new WatcherProcessFailure( + `file watcher subscription timed out after ${record.hooks.subscribeTimeoutMs}ms`, + 'subscription', + 'subscribe_timeout' + ) + ) + }, record.hooks.subscribeTimeoutMs) + pending.timer.unref?.() +} + +export function startInterruptedSubscribeTimeout( + record: WatcherProcessSubscriptionRecord, + cancel: (error: WatcherProcessFailure) => void +): void { + if ( + !record.interrupted || + record.pendingSubscribe || + record.resubscribeTimer || + record.hooks.subscribeTimeoutMs === undefined + ) { + return + } + record.crawlStarted = true + record.resubscribeTimer = setTimeout(() => { + cancel( + new WatcherProcessFailure( + `file watcher resubscription timed out after ${record.hooks.subscribeTimeoutMs}ms`, + 'subscription', + 'subscribe_timeout' + ) + ) + }, record.hooks.subscribeTimeoutMs) + record.resubscribeTimer.unref?.() +} + +export function resetPendingSubscribeAttempt(record: WatcherProcessSubscriptionRecord): void { + record.crawlStarted = false + if (record.resubscribeTimer) { + clearTimeout(record.resubscribeTimer) + record.resubscribeTimer = undefined + } + const pending = record.pendingSubscribe + if (pending?.timer) { + clearTimeout(pending.timer) + pending.timer = undefined + } +} + +export function takePendingSubscribe( + record: WatcherProcessSubscriptionRecord +): PendingWatcherProcessSubscribe | undefined { + const pending = record.pendingSubscribe + if (!pending) { + return undefined + } + record.pendingSubscribe = undefined + if (pending.abortListener && record.hooks.signal) { + record.hooks.signal.removeEventListener('abort', pending.abortListener) + } + if (pending.timer) { + clearTimeout(pending.timer) + } + return pending +} diff --git a/src/main/ipc/parcel-watcher-process-entry.test.ts b/src/main/ipc/parcel-watcher-process-entry.test.ts index 13f808ab650..19740f17fff 100644 --- a/src/main/ipc/parcel-watcher-process-entry.test.ts +++ b/src/main/ipc/parcel-watcher-process-entry.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const { subscribeMock, writeFileSyncMock } = vi.hoisted(() => ({ +const { statMock, subscribeMock, writeFileSyncMock } = vi.hoisted(() => ({ + statMock: vi.fn(), subscribeMock: vi.fn(), writeFileSyncMock: vi.fn() })) @@ -10,21 +11,25 @@ vi.mock('node:fs', () => ({ rmSync: vi.fn(), writeFileSync: writeFileSyncMock })) +vi.mock('node:fs/promises', () => ({ stat: statMock })) vi.mock('@parcel/watcher', () => ({ subscribe: subscribeMock })) describe('parcel watcher process canary', () => { let originalMessageListeners: ReturnType let originalExitListeners: ReturnType let originalDisconnectListeners: ReturnType + let originalSend: typeof process.send beforeEach(() => { vi.useFakeTimers() vi.resetModules() subscribeMock.mockReset() + statMock.mockReset() writeFileSyncMock.mockReset() originalMessageListeners = process.listeners('message') originalExitListeners = process.listeners('exit') originalDisconnectListeners = process.listeners('disconnect') + originalSend = process.send }) afterEach(() => { @@ -43,6 +48,7 @@ describe('parcel watcher process canary', () => { process.off('disconnect', listener) } } + process.send = originalSend vi.useRealTimers() vi.restoreAllMocks() }) @@ -201,6 +207,80 @@ describe('parcel watcher process canary', () => { expect(exitSpy).toHaveBeenCalledWith(2) }) + it('removes a cancelled queued crawl before it reaches Parcel', async () => { + let finishActiveCrawl: + | ((subscription: { unsubscribe: () => Promise }) => void) + | undefined + subscribeMock.mockResolvedValueOnce({ unsubscribe: vi.fn() }).mockReturnValueOnce( + new Promise((resolve) => { + finishActiveCrawl = resolve + }) + ) + const sendMock = vi.fn() + process.send = sendMock + + await import('./parcel-watcher-process-entry') + await vi.advanceTimersByTimeAsync(0) + process.emit('message', { op: 'subscribe', id: 1, dir: '/active', opts: {} }) + await vi.advanceTimersByTimeAsync(0) + process.emit('message', { op: 'subscribe', id: 2, dir: '/queued', opts: {} }) + process.emit('message', { op: 'cancel-subscribe', id: 2 }) + + // Why: non-restart cancel reuses async unsubscribe teardown; drain the + // exclusive lifecycle queue after the active crawl finishes. + finishActiveCrawl?.({ unsubscribe: vi.fn().mockResolvedValue(undefined) }) + await vi.advanceTimersByTimeAsync(0) + + expect(sendMock).toHaveBeenCalledWith({ op: 'unsubscribed', id: 2 }) + expect(subscribeMock).toHaveBeenCalledTimes(2) + expect(sendMock).not.toHaveBeenCalledWith({ op: 'subscribe-started', id: 2 }) + expect(sendMock).not.toHaveBeenCalledWith({ op: 'subscribed', id: 2 }) + }) + + it('unsubscribes a late cancel after the crawl already finished', async () => { + const unsubscribe = vi.fn().mockResolvedValue(undefined) + subscribeMock.mockResolvedValueOnce({ unsubscribe: vi.fn() }).mockResolvedValueOnce({ + unsubscribe + }) + const sendMock = vi.fn() + process.send = sendMock + + await import('./parcel-watcher-process-entry') + await vi.advanceTimersByTimeAsync(0) + process.emit('message', { op: 'subscribe', id: 1, dir: '/finished', opts: {} }) + await vi.advanceTimersByTimeAsync(0) + + expect(sendMock).toHaveBeenCalledWith({ op: 'subscribed', id: 1 }) + process.emit('message', { op: 'cancel-subscribe', id: 1 }) + await vi.advanceTimersByTimeAsync(0) + + expect(unsubscribe).toHaveBeenCalledTimes(1) + expect(sendMock).toHaveBeenCalledWith({ op: 'unsubscribed', id: 1 }) + expect(sendMock).not.toHaveBeenCalledWith({ op: 'cancel-requires-restart', id: 1 }) + }) + + it('asks the host to restart when an active crawl is cancelled', async () => { + let finishCrawl: ((subscription: { unsubscribe: () => Promise }) => void) | undefined + subscribeMock.mockResolvedValueOnce({ unsubscribe: vi.fn() }).mockReturnValueOnce( + new Promise((resolve) => { + finishCrawl = resolve + }) + ) + const sendMock = vi.fn() + process.send = sendMock + + await import('./parcel-watcher-process-entry') + await vi.advanceTimersByTimeAsync(0) + process.emit('message', { op: 'subscribe', id: 1, dir: '/active', opts: {} }) + await vi.advanceTimersByTimeAsync(0) + process.emit('message', { op: 'cancel-subscribe', id: 1 }) + + expect(sendMock).toHaveBeenCalledWith({ op: 'cancel-requires-restart', id: 1 }) + finishCrawl?.({ unsubscribe: vi.fn().mockResolvedValue(undefined) }) + await vi.advanceTimersByTimeAsync(0) + expect(sendMock).not.toHaveBeenCalledWith({ op: 'subscribed', id: 1 }) + }) + it('still restarts after consecutive missed events once every subscription is live', async () => { subscribeMock.mockResolvedValue({ unsubscribe: vi.fn() }) const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => undefined) as never) @@ -215,4 +295,198 @@ describe('parcel watcher process canary', () => { expect(writeFileSyncMock).toHaveBeenCalledTimes(3) expect(exitSpy).toHaveBeenCalledWith(2) }) + + it('collapses and enriches runtime batches before they cross child IPC', async () => { + let callback: + | ((err: Error | null, events: { type: string; path: string }[]) => void) + | undefined + subscribeMock + .mockResolvedValueOnce({ unsubscribe: vi.fn() }) + .mockImplementationOnce(async (_dir, nextCallback) => { + callback = nextCallback + return { unsubscribe: vi.fn() } + }) + statMock.mockImplementation(async (eventPath: string) => ({ + isDirectory: () => eventPath.endsWith('/dir') + })) + const sendMock = vi.fn() + process.send = sendMock + + await import('./parcel-watcher-process-entry') + await vi.advanceTimersByTimeAsync(0) + process.emit('message', { + op: 'subscribe', + id: 1, + dir: '/repo', + opts: {}, + delivery: { includeDirectoryMetadata: true, maxEventsPerBatch: 2 } + }) + await vi.advanceTimersByTimeAsync(0) + + callback?.(null, [ + { type: 'update', path: '/repo/file.txt' }, + { type: 'create', path: '/repo/dir' } + ]) + await vi.advanceTimersByTimeAsync(0) + expect(sendMock).toHaveBeenCalledWith( + { + op: 'events', + id: 1, + events: [ + { type: 'update', path: '/repo/file.txt', isDirectory: false }, + { type: 'create', path: '/repo/dir', isDirectory: true } + ] + }, + expect.any(Function) + ) + + sendMock.mockClear() + callback?.(null, [ + { type: 'update', path: '/repo/a' }, + { type: 'update', path: '/repo/b' }, + { type: 'update', path: '/repo/c' } + ]) + await vi.advanceTimersByTimeAsync(0) + expect(sendMock).toHaveBeenCalledWith({ op: 'overflow', id: 1 }, expect.any(Function)) + expect(statMock).toHaveBeenCalledTimes(2) + }) + + it('reports an FSEvents overflow and keeps delivering later events', async () => { + let callback: + | ((err: Error | null, events: { type: string; path: string }[]) => void) + | undefined + subscribeMock + .mockResolvedValueOnce({ unsubscribe: vi.fn() }) + .mockImplementationOnce(async (_dir, nextCallback) => { + callback = nextCallback + return { unsubscribe: vi.fn() } + }) + const sendMock = vi.fn() + process.send = sendMock + + await import('./parcel-watcher-process-entry') + await vi.advanceTimersByTimeAsync(0) + process.emit('message', { op: 'subscribe', id: 1, dir: '/repo', opts: {} }) + await vi.advanceTimersByTimeAsync(0) + + callback?.( + new Error('Events were dropped by the FSEvents client. File system must be re-scanned.'), + [] + ) + callback?.(null, [{ type: 'update', path: '/repo/after-overflow.txt' }]) + await vi.advanceTimersByTimeAsync(0) + + expect(sendMock).toHaveBeenCalledWith({ + op: 'watch-error', + id: 1, + message: 'Events were dropped by the FSEvents client. File system must be re-scanned.' + }) + expect(sendMock).toHaveBeenCalledWith( + { + op: 'events', + id: 1, + events: [{ type: 'update', path: '/repo/after-overflow.txt' }] + }, + expect.any(Function) + ) + }) + + it('bounds pending event batches while child IPC reports backpressure', async () => { + let callback: + | ((err: Error | null, events: { type: string; path: string }[]) => void) + | undefined + let releaseFirstEvent: (() => void) | undefined + const sentEventPaths: string[] = [] + subscribeMock + .mockResolvedValueOnce({ unsubscribe: vi.fn() }) + .mockImplementationOnce(async (_dir, nextCallback) => { + callback = nextCallback + return { unsubscribe: vi.fn() } + }) + process.send = vi.fn((message, onSent) => { + if (message.op !== 'events') { + return true + } + sentEventPaths.push(message.events[0]?.path ?? '') + if (!releaseFirstEvent) { + releaseFirstEvent = onSent + return false + } + return true + }) as typeof process.send + + await import('./parcel-watcher-process-entry') + await vi.advanceTimersByTimeAsync(0) + process.emit('message', { + op: 'subscribe', + id: 1, + dir: '/repo', + opts: {}, + delivery: { includeDirectoryMetadata: true, maxEventsPerBatch: 2 } + }) + await vi.advanceTimersByTimeAsync(0) + + callback?.(null, [{ type: 'update', path: '/repo/first.txt' }]) + callback?.(null, [{ type: 'update', path: '/repo/second.txt' }]) + callback?.(null, [{ type: 'update', path: '/repo/third.txt' }]) + callback?.(null, [{ type: 'update', path: '/repo/fourth.txt' }]) + await vi.advanceTimersByTimeAsync(0) + expect(sentEventPaths).toEqual(['/repo/first.txt']) + + releaseFirstEvent?.() + await vi.advanceTimersByTimeAsync(0) + expect(sentEventPaths).toEqual(['/repo/first.txt']) + expect(process.send).toHaveBeenCalledWith({ op: 'overflow', id: 1 }, expect.any(Function)) + expect(statMock).toHaveBeenCalledTimes(1) + }) + + it('drops queued event work when the subscription is removed', async () => { + let callback: + | ((err: Error | null, events: { type: string; path: string }[]) => void) + | undefined + let releaseFirstEvent: (() => void) | undefined + const unsubscribe = vi.fn().mockResolvedValue(undefined) + subscribeMock + .mockResolvedValueOnce({ unsubscribe: vi.fn() }) + .mockImplementationOnce(async (_dir, nextCallback) => { + callback = nextCallback + return { unsubscribe } + }) + process.send = vi.fn((message, onSent) => { + if (message.op === 'events' && !releaseFirstEvent) { + releaseFirstEvent = onSent + return false + } + return true + }) as typeof process.send + + await import('./parcel-watcher-process-entry') + await vi.advanceTimersByTimeAsync(0) + process.emit('message', { + op: 'subscribe', + id: 1, + dir: '/repo', + opts: {}, + delivery: { includeDirectoryMetadata: true, maxEventsPerBatch: 2 } + }) + await vi.advanceTimersByTimeAsync(0) + + callback?.(null, [{ type: 'update', path: '/repo/first.txt' }]) + callback?.(null, [{ type: 'update', path: '/repo/queued.txt' }]) + await vi.advanceTimersByTimeAsync(0) + process.emit('message', { op: 'unsubscribe', id: 1 }) + await vi.advanceTimersByTimeAsync(0) + releaseFirstEvent?.() + await vi.advanceTimersByTimeAsync(0) + + expect(unsubscribe).toHaveBeenCalledTimes(1) + expect(statMock).toHaveBeenCalledTimes(1) + expect(process.send).not.toHaveBeenCalledWith( + expect.objectContaining({ + op: 'events', + events: [expect.objectContaining({ path: '/repo/queued.txt' })] + }), + expect.any(Function) + ) + }) }) diff --git a/src/main/ipc/parcel-watcher-process-entry.ts b/src/main/ipc/parcel-watcher-process-entry.ts index 6a296eb2af9..1e043739a4a 100644 --- a/src/main/ipc/parcel-watcher-process-entry.ts +++ b/src/main/ipc/parcel-watcher-process-entry.ts @@ -1,4 +1,4 @@ -// Forked (ELECTRON_RUN_AS_NODE) child that hosts all local @parcel/watcher +// Forked (ELECTRON_RUN_AS_NODE) child that hosts its assigned @parcel/watcher // subscriptions. Why: watcher.node has native teardown races that fail-fast // the hosting process (issue #7547, 0xc0000409 on Windows; same class as // #5377/#6635). Running the native module here turns a watcher fault into a @@ -8,28 +8,16 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import type * as ParcelWatcher from '@parcel/watcher' - -export type WatcherProcessEvent = { - type: 'create' | 'update' | 'delete' - path: string -} - -export type WatcherProcessSubscribeOptions = { - ignore?: string[] - ignoreGlobs?: string[] - backend?: string -} - -export type HostToWatcherMessage = - | { op: 'subscribe'; id: number; dir: string; opts: WatcherProcessSubscribeOptions } - | { op: 'unsubscribe'; id: number } - -export type WatcherToHostMessage = - | { op: 'subscribed'; id: number } - | { op: 'subscribe-failed'; id: number; message: string } - | { op: 'events'; id: number; events: WatcherProcessEvent[] } - | { op: 'watch-error'; id: number; message: string } - | { op: 'unsubscribed'; id: number } +import { + createWatcherProcessEventDeliveryQueue, + type WatcherProcessEventDeliveryQueue +} from './parcel-watcher-event-delivery' +import type { + HostToWatcherMessage, + WatcherProcessDeliveryOptions, + WatcherProcessSubscribeOptions, + WatcherToHostMessage +} from './parcel-watcher-process-protocol' function errorMessage(err: unknown): string { return err instanceof Error ? err.message : String(err) @@ -49,8 +37,9 @@ const CANARY_MAX_MISSES = 2 async function startCanary(getStableActivityRevision: () => number | null): Promise { let canaryDir: string let lastEventAt = 0 + const configuredCanaryDir = process.env.ORCA_WATCHER_CANARY_DIR try { - canaryDir = mkdtempSync(join(tmpdir(), 'orca-watcher-canary-')) + canaryDir = configuredCanaryDir ?? mkdtempSync(join(tmpdir(), 'orca-watcher-canary-')) const watcher = await import('@parcel/watcher') // Why: pin the Windows backend like the main subscriptions do, so the // canary never probes for Watchman. @@ -71,6 +60,9 @@ async function startCanary(getStableActivityRevision: () => number | null): Prom return } process.on('exit', () => { + if (configuredCanaryDir) { + return + } try { rmSync(canaryDir, { recursive: true, force: true }) } catch { @@ -125,12 +117,34 @@ function main(): void { } } + const sendEventWithBackpressure = (message: WatcherToHostMessage): Promise => { + return new Promise((resolve) => { + let settled = false + const finish = (): void => { + if (!settled) { + settled = true + resolve() + } + } + try { + const accepted = process.send?.(message, finish) + if (accepted !== false) { + finish() + } + } catch { + finish() + } + }) + } + // Subscribe promises are kept (not just subscriptions) so an unsubscribe // that races a still-crawling subscribe awaits it instead of leaking the // native handle — on Windows a leaked handle keeps the worktree dir locked. const subscriptions = new Map>() const liveSubscriptionIds = new Set() + const eventDeliveries = new Map() let pendingSubscriptionCrawls = 0 + let activeSubscriptionCrawlId: number | null = null let subscriptionActivityRevision = 0 let nativeLifecycleTail = Promise.resolve() @@ -156,11 +170,29 @@ function main(): void { const handleSubscribe = async ( id: number, dir: string, - opts: WatcherProcessSubscribeOptions + opts: WatcherProcessSubscribeOptions, + delivery: WatcherProcessDeliveryOptions | undefined ): Promise => { + const eventDelivery = createWatcherProcessEventDeliveryQueue( + delivery, + async (events) => { + await sendEventWithBackpressure( + events === null ? { op: 'overflow', id } : { op: 'events', id, events } + ) + }, + (deliveryError) => { + send({ op: 'watch-error', id, message: errorMessage(deliveryError) }) + } + ) + eventDeliveries.set(id, eventDelivery) try { const subscription = await runNativeWatcherLifecycleExclusive(async () => { + if (!subscriptions.has(id)) { + return null + } + activeSubscriptionCrawlId = id beginSubscriptionCrawl() + send({ op: 'subscribe-started', id }) try { const watcher = await import('@parcel/watcher') return await watcher.subscribe( @@ -170,31 +202,32 @@ function main(): void { send({ op: 'watch-error', id, message: errorMessage(err) }) return } - if (events.length > 0) { - send({ - op: 'events', - id, - events: events.map((event) => ({ type: event.type, path: event.path })) - }) - } + eventDelivery.enqueue(events) }, opts as ParcelWatcher.Options ) } finally { + activeSubscriptionCrawlId = null finishSubscriptionCrawl() } }) // An unsubscribe can remove the record while subscribe() is crawling. // Only advertise it as live if it is still owned by this process. - if (subscriptions.has(id)) { + const stillOwned = subscriptions.has(id) + if (stillOwned) { liveSubscriptionIds.add(id) + send({ op: 'subscribed', id }) } - send({ op: 'subscribed', id }) return subscription } catch (err) { + const stillOwned = subscriptions.has(id) + eventDelivery.close() + eventDeliveries.delete(id) subscriptions.delete(id) liveSubscriptionIds.delete(id) - send({ op: 'subscribe-failed', id, message: errorMessage(err) }) + if (stillOwned) { + send({ op: 'subscribe-failed', id, message: errorMessage(err) }) + } return null } } @@ -206,6 +239,8 @@ function main(): void { const pending = subscriptions.get(id) subscriptions.delete(id) liveSubscriptionIds.delete(id) + eventDeliveries.get(id)?.close() + eventDeliveries.delete(id) try { // Why: Parcel already serializes crawl and teardown on one backend mutex. // Mirror that ordering here so neither operation can mask a canary miss. @@ -223,16 +258,40 @@ function main(): void { send({ op: 'unsubscribed', id }) } + const handleCancelSubscribe = (id: number): void => { + // Why: an in-flight native crawl cannot be cancelled safely; ask the host to + // kill this child instead of risking the Parcel teardown deadlock. + if (activeSubscriptionCrawlId === id) { + subscriptions.delete(id) + liveSubscriptionIds.delete(id) + eventDeliveries.get(id)?.close() + eventDeliveries.delete(id) + send({ op: 'cancel-requires-restart', id }) + return + } + // Why: cancel can race a crawl that already finished. Reuse unsubscribe + // teardown so a live native handle is released instead of leaked (Windows + // keeps the worktree locked while the handle stays open). + void handleUnsubscribe(id) + } + process.on('message', (message: HostToWatcherMessage) => { if (!message || typeof message !== 'object') { return } if (message.op === 'subscribe') { - subscriptions.set(message.id, handleSubscribe(message.id, message.dir, message.opts)) + subscriptions.set( + message.id, + handleSubscribe(message.id, message.dir, message.opts, message.delivery) + ) return } if (message.op === 'unsubscribe') { void handleUnsubscribe(message.id) + return + } + if (message.op === 'cancel-subscribe') { + handleCancelSubscribe(message.id) } }) diff --git a/src/main/ipc/parcel-watcher-process-failure.ts b/src/main/ipc/parcel-watcher-process-failure.ts new file mode 100644 index 00000000000..b93f2f37578 --- /dev/null +++ b/src/main/ipc/parcel-watcher-process-failure.ts @@ -0,0 +1,25 @@ +export type WatcherProcessFailureScope = 'subscription' | 'supervisor' + +export type WatcherProcessFailureCode = + | 'entry_missing' + | 'process_unavailable' + | 'subscribe_aborted' + | 'subscribe_failed' + | 'subscribe_timeout' + | 'supervisor_crash_fuse' + | 'supervisor_disposed' + +export class WatcherProcessFailure extends Error { + constructor( + message: string, + readonly scope: WatcherProcessFailureScope, + readonly code: WatcherProcessFailureCode + ) { + super(message) + this.name = 'WatcherProcessFailure' + } +} + +export function isWatcherProcessFailure(error: unknown): error is WatcherProcessFailure { + return error instanceof WatcherProcessFailure +} diff --git a/src/main/ipc/parcel-watcher-process-protocol.ts b/src/main/ipc/parcel-watcher-process-protocol.ts new file mode 100644 index 00000000000..17dbcc36a88 --- /dev/null +++ b/src/main/ipc/parcel-watcher-process-protocol.ts @@ -0,0 +1,37 @@ +export type WatcherProcessEvent = { + type: 'create' | 'update' | 'delete' + path: string + isDirectory?: boolean +} + +export type WatcherProcessSubscribeOptions = { + ignore?: string[] + ignoreGlobs?: string[] + backend?: string +} + +export type WatcherProcessDeliveryOptions = { + includeDirectoryMetadata?: boolean + maxEventsPerBatch?: number +} + +export type HostToWatcherMessage = + | { + op: 'subscribe' + id: number + dir: string + opts: WatcherProcessSubscribeOptions + delivery?: WatcherProcessDeliveryOptions + } + | { op: 'unsubscribe'; id: number } + | { op: 'cancel-subscribe'; id: number } + +export type WatcherToHostMessage = + | { op: 'subscribe-started'; id: number } + | { op: 'subscribed'; id: number } + | { op: 'subscribe-failed'; id: number; message: string } + | { op: 'events'; id: number; events: WatcherProcessEvent[] } + | { op: 'overflow'; id: number } + | { op: 'watch-error'; id: number; message: string } + | { op: 'cancel-requires-restart'; id: number } + | { op: 'unsubscribed'; id: number } diff --git a/src/main/ipc/parcel-watcher-process-subscription.ts b/src/main/ipc/parcel-watcher-process-subscription.ts new file mode 100644 index 00000000000..54489b86397 --- /dev/null +++ b/src/main/ipc/parcel-watcher-process-subscription.ts @@ -0,0 +1,35 @@ +import type { + WatcherProcessDeliveryOptions, + WatcherProcessEvent, + WatcherProcessSubscribeOptions +} from './parcel-watcher-process-protocol' + +export type WatcherProcessCallback = (err: Error | null, events: WatcherProcessEvent[]) => void +export type WatcherProcessSubscription = { unsubscribe(): Promise } +export type WatcherProcessHooks = { + delivery?: WatcherProcessDeliveryOptions + onInterruption?: () => void + onOverflow?: () => void + onTerminalError?: (error: Error) => void + signal?: AbortSignal + subscribeTimeoutMs?: number +} + +export type PendingWatcherProcessSubscribe = { + resolve: () => void + reject: (err: Error) => void + abortListener?: () => void + timer?: ReturnType +} + +export type WatcherProcessSubscriptionRecord = { + id: number + dir: string + opts: WatcherProcessSubscribeOptions + callback: WatcherProcessCallback + hooks: WatcherProcessHooks + interrupted: boolean + crawlStarted: boolean + resubscribeTimer?: ReturnType + pendingSubscribe?: PendingWatcherProcessSubscribe +} diff --git a/src/main/ipc/parcel-watcher-process-supervisor.ts b/src/main/ipc/parcel-watcher-process-supervisor.ts new file mode 100644 index 00000000000..40a9c193b4c --- /dev/null +++ b/src/main/ipc/parcel-watcher-process-supervisor.ts @@ -0,0 +1,316 @@ +import type { ChildProcess } from 'node:child_process' +import { existsSync } from 'node:fs' +import { getWatcherProcessEntryPath } from './parcel-watcher-entry-path' +import { removeWatcherCanaryDirectory } from './parcel-watcher-canary-directory' +import { launchWatcherChild } from './parcel-watcher-child-launch' +import { WatcherProcessCrashFuse } from './parcel-watcher-crash-fuse' +import { + failAllWatcherSubscriptions, + handleWatcherHostMessage, + reportWatcherTerminalError, + resolvePendingWatcherUnsubscribes +} from './parcel-watcher-host-subscriptions' +import { + resetPendingSubscribeAttempt, + startInterruptedSubscribeTimeout, + startPendingSubscribeTimeout, + takePendingSubscribe +} from './parcel-watcher-pending-subscribe' +import { WatcherProcessFailure } from './parcel-watcher-process-failure' +import type { + HostToWatcherMessage, + WatcherProcessSubscribeOptions, + WatcherToHostMessage +} from './parcel-watcher-process-protocol' +import type { + WatcherProcessCallback, + WatcherProcessHooks, + WatcherProcessSubscription, + WatcherProcessSubscriptionRecord +} from './parcel-watcher-process-subscription' +import { subscribeThroughWatcherSupervisor } from './parcel-watcher-supervisor-subscribe' + +export type { + WatcherProcessCallback, + WatcherProcessHooks, + WatcherProcessSubscription +} from './parcel-watcher-process-subscription' + +export class WatcherProcessSupervisor { + private child: ChildProcess | null = null + private nextSubscriptionId = 1 + private readonly crashFuse = new WatcherProcessCrashFuse() + private shutdownRequested = false + private canaryDir: string | null = null + private readonly records = new Map() + private readonly pendingUnsubscribes = new Map void>() + private readonly cancelledSubscribesAwaitingChild = new Set() + subscribe( + dir: string, + callback: WatcherProcessCallback, + opts: WatcherProcessSubscribeOptions, + hooks: WatcherProcessHooks = {} + ): Promise { + return subscribeThroughWatcherSupervisor({ + dir, + callback, + opts, + hooks, + shutdownRequested: this.shutdownRequested, + allocateId: () => this.nextSubscriptionId++, + records: this.records, + pendingUnsubscribes: this.pendingUnsubscribes, + ensureWatcherProcess: (entryPath) => this.ensureWatcherProcess(entryPath), + getChild: () => this.child, + killWatcherChildIfIdle: () => this.killWatcherChildIfIdle(), + sendSubscribe: (child, record) => this.sendSubscribe(child, record), + sendToChild: (child, message) => this.sendToChild(child, message), + cancelPendingSubscribe: (record, error) => this.cancelPendingSubscribe(record, error) + }) + } + + dispose(): void { + this.shutdownRequested = true + const proc = this.child + this.child = null + const error = new WatcherProcessFailure( + 'file watcher supervisor disposed', + 'supervisor', + 'supervisor_disposed' + ) + for (const record of this.records.values()) { + resetPendingSubscribeAttempt(record) + const pending = takePendingSubscribe(record) + pending?.reject(error) + } + resolvePendingWatcherUnsubscribes(this.pendingUnsubscribes) + this.cancelledSubscribesAwaitingChild.clear() + this.records.clear() + proc?.kill() + this.canaryDir = removeWatcherCanaryDirectory(this.canaryDir) + } + + resetForTest(): void { + this.dispose() + this.shutdownRequested = false + this.crashFuse.reset() + } + + private ensureWatcherProcess(entryPath = getWatcherProcessEntryPath()): ChildProcess | null { + if (this.shutdownRequested) { + return null + } + if (this.child?.connected) { + return this.child + } + if (this.crashFuse.isOpen()) { + return null + } + if (!existsSync(entryPath)) { + console.error(`[parcel-watcher-process] entry not found at ${entryPath}; refusing fail-open`) + return null + } + const launched = launchWatcherChild( + entryPath, + this.canaryDir, + (child, message) => { + if (this.child === child) { + this.handleChildMessage(message) + } + }, + (child, code, signal) => this.handleChildGone(child, code, signal) + ) + if (!launched) { + this.canaryDir = null + return null + } + this.canaryDir = launched.canaryDir + this.child = launched.child + return launched.child + } + + private handleChildMessage(message: WatcherToHostMessage): void { + if (message.op === 'subscribe-started') { + const record = this.records.get(message.id) + if (record) { + startPendingSubscribeTimeout(record, (error) => this.cancelPendingSubscribe(record, error)) + startInterruptedSubscribeTimeout(record, (error) => + this.cancelInterruptedSubscribe(record, error) + ) + } + return + } + if (message.op === 'cancel-requires-restart') { + if (this.cancelledSubscribesAwaitingChild.delete(message.id)) { + this.restartAfterCancelledSubscribe() + } + return + } + if (message.op === 'unsubscribed') { + const completedCancellation = this.cancelledSubscribesAwaitingChild.delete(message.id) + handleWatcherHostMessage( + message, + this.records, + this.pendingUnsubscribes, + reportWatcherTerminalError, + () => this.killWatcherChildIfIdle() + ) + if (completedCancellation) { + this.killWatcherChildIfIdle() + } + return + } + handleWatcherHostMessage( + message, + this.records, + this.pendingUnsubscribes, + reportWatcherTerminalError, + () => this.killWatcherChildIfIdle() + ) + } + + private handleChildGone( + proc: ChildProcess, + code?: number | null, + signal?: NodeJS.Signals | null + ): void { + if (this.child !== proc) { + return + } + if (code !== undefined && (code !== 0 || signal)) { + console.error( + `[parcel-watcher-process] watcher process exited (code=${code}, signal=${signal})` + ) + } + this.child = null + this.cancelledSubscribesAwaitingChild.clear() + resolvePendingWatcherUnsubscribes(this.pendingUnsubscribes) + if (this.shutdownRequested || this.records.size === 0) { + return + } + this.crashFuse.recordCrash() + for (const record of this.records.values()) { + record.interrupted = true + resetPendingSubscribeAttempt(record) + } + const replacement = this.ensureWatcherProcess() + if (!replacement) { + console.error( + '[parcel-watcher-process] watcher process crashed repeatedly; disabling file watching' + ) + failAllWatcherSubscriptions( + this.records, + new WatcherProcessFailure( + 'file watcher process crashed repeatedly', + 'supervisor', + 'supervisor_crash_fuse' + ) + ) + this.canaryDir = removeWatcherCanaryDirectory(this.canaryDir) + return + } + console.error( + `[parcel-watcher-process] watcher process crashed; resubscribing ${this.records.size} root(s)` + ) + for (const record of this.records.values()) { + this.sendSubscribe(replacement, record) + } + } + + private killWatcherChildIfIdle(): void { + const proc = this.child + if (!proc || this.records.size > 0) { + return + } + this.child = null + resolvePendingWatcherUnsubscribes(this.pendingUnsubscribes) + proc.kill() + this.canaryDir = removeWatcherCanaryDirectory(this.canaryDir) + } + + private cancelPendingSubscribe( + record: WatcherProcessSubscriptionRecord, + error: WatcherProcessFailure + ): void { + if (!record.pendingSubscribe || !this.records.delete(record.id)) { + return + } + const crawlStarted = record.crawlStarted + const pending = takePendingSubscribe(record) + pending?.reject(error) + if (crawlStarted) { + this.restartAfterCancelledSubscribe() + return + } + const proc = this.child + if (!proc?.connected) { + this.killWatcherChildIfIdle() + return + } + this.cancelledSubscribesAwaitingChild.add(record.id) + this.sendToChild(proc, { op: 'cancel-subscribe', id: record.id }) + } + + private cancelInterruptedSubscribe( + record: WatcherProcessSubscriptionRecord, + error: WatcherProcessFailure + ): void { + if (!record.interrupted || record.pendingSubscribe || !this.records.delete(record.id)) { + return + } + resetPendingSubscribeAttempt(record) + // Why: one slow recovery root must not pin every healthy root in its shard. + // End that subscription so the runtime pool can retry it in quarantine. + reportWatcherTerminalError(record, error) + this.restartAfterCancelledSubscribe() + } + + private restartAfterCancelledSubscribe(): void { + const proc = this.child + this.child = null + this.cancelledSubscribesAwaitingChild.clear() + resolvePendingWatcherUnsubscribes(this.pendingUnsubscribes) + proc?.kill() + if (this.records.size === 0) { + this.canaryDir = removeWatcherCanaryDirectory(this.canaryDir) + return + } + const replacement = this.ensureWatcherProcess() + if (!replacement) { + failAllWatcherSubscriptions( + this.records, + new WatcherProcessFailure( + 'file watcher process unavailable after subscription cancellation', + 'supervisor', + 'process_unavailable' + ) + ) + this.canaryDir = removeWatcherCanaryDirectory(this.canaryDir) + return + } + for (const liveRecord of this.records.values()) { + liveRecord.interrupted = true + resetPendingSubscribeAttempt(liveRecord) + this.sendSubscribe(replacement, liveRecord) + } + } + + private sendSubscribe(proc: ChildProcess, record: WatcherProcessSubscriptionRecord): void { + resetPendingSubscribeAttempt(record) + this.sendToChild(proc, { + op: 'subscribe', + id: record.id, + dir: record.dir, + opts: record.opts, + delivery: record.hooks.delivery + }) + } + + private sendToChild(proc: ChildProcess, message: HostToWatcherMessage): void { + try { + proc.send(message) + } catch { + // Channel already closed — the child's exit handler recovers. + } + } +} diff --git a/src/main/ipc/parcel-watcher-process.test.ts b/src/main/ipc/parcel-watcher-process.test.ts index 4df3b666fbb..27891840012 100644 --- a/src/main/ipc/parcel-watcher-process.test.ts +++ b/src/main/ipc/parcel-watcher-process.test.ts @@ -4,19 +4,30 @@ import { EventEmitter } from 'node:events' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const { forkMock, existsSyncMock, parcelSubscribeMock } = vi.hoisted(() => ({ - forkMock: vi.fn(), - existsSyncMock: vi.fn(), - parcelSubscribeMock: vi.fn() -})) +const { forkMock, existsSyncMock, mkdtempSyncMock, parcelSubscribeMock, rmSyncMock } = vi.hoisted( + () => ({ + forkMock: vi.fn(), + existsSyncMock: vi.fn(), + mkdtempSyncMock: vi.fn(() => '/tmp/orca-watcher-canary-supervisor-test'), + parcelSubscribeMock: vi.fn(), + rmSyncMock: vi.fn() + }) +) vi.mock('node:child_process', () => ({ fork: forkMock })) -vi.mock('node:fs', () => ({ existsSync: existsSyncMock })) +vi.mock('node:fs', () => ({ + existsSync: existsSyncMock, + mkdtempSync: mkdtempSyncMock, + rmSync: rmSyncMock +})) vi.mock('@parcel/watcher', () => ({ subscribe: parcelSubscribeMock })) import { + createWatcherProcessSupervisor, disposeWatcherProcess, + resetRuntimeWatcherProcessForTest, resetWatcherProcessForTest, + subscribeViaRuntimeWatcherProcess, subscribeViaWatcherProcess } from './parcel-watcher-process' @@ -55,6 +66,7 @@ function ackSubscribe(child: FakeChild, index = -1): number { describe('subscribeViaWatcherProcess', () => { beforeEach(() => { resetWatcherProcessForTest() + resetRuntimeWatcherProcessForTest() // Why: the client deliberately runs in-process under vitest; these tests // exercise the forked-process mode, so hide the vitest marker. vi.stubEnv('VITEST', '') @@ -64,6 +76,7 @@ describe('subscribeViaWatcherProcess', () => { afterEach(() => { disposeWatcherProcess() + resetRuntimeWatcherProcessForTest() vi.unstubAllEnvs() vi.clearAllMocks() }) @@ -92,6 +105,42 @@ describe('subscribeViaWatcherProcess', () => { expect(callback).toHaveBeenCalledWith(expect.objectContaining({ message: 'boom' }), []) }) + it('reports a terminal resubscribe failure separately from recoverable watch errors', async () => { + const callback = vi.fn() + const onTerminalError = vi.fn() + const promise = subscribeViaWatcherProcess('/repo', callback, {}, { onTerminalError }) + const child = currentChild() + const id = ackSubscribe(child) + await promise + + child.emit('message', { op: 'subscribe-failed', id, message: 'root unavailable' }) + + expect(onTerminalError).toHaveBeenCalledWith( + expect.objectContaining({ message: 'root unavailable' }) + ) + expect(callback).not.toHaveBeenCalled() + }) + + it('passes bounded event-delivery options to the watcher child', async () => { + const promise = subscribeViaWatcherProcess( + '/repo', + vi.fn(), + {}, + { + delivery: { includeDirectoryMetadata: true, maxEventsPerBatch: 200 } + } + ) + const child = currentChild() + + expect(child.sent[0]).toMatchObject({ + op: 'subscribe', + dir: '/repo', + delivery: { includeDirectoryMetadata: true, maxEventsPerBatch: 200 } + }) + ackSubscribe(child) + await promise + }) + it('rejects the subscribe when the watcher process reports failure', async () => { const promise = subscribeViaWatcherProcess('/gone', vi.fn(), {}) const child = currentChild() @@ -135,7 +184,7 @@ describe('subscribeViaWatcherProcess', () => { it('respawns after a crash, resubscribes, and reports the interruption', async () => { const callback = vi.fn() const onInterruption = vi.fn() - const promise = subscribeViaWatcherProcess('/repo', callback, {}, onInterruption) + const promise = subscribeViaWatcherProcess('/repo', callback, {}, { onInterruption }) const first = currentChild() const id = ackSubscribe(first) await promise @@ -157,8 +206,71 @@ describe('subscribeViaWatcherProcess', () => { expect(callback).toHaveBeenCalledWith(null, events) }) + it('recovers existing records when IPC disconnects before child exit', async () => { + const firstPromise = subscribeViaWatcherProcess('/existing', vi.fn(), {}) + const first = currentChild() + ackSubscribe(first) + await firstPromise + + first.connected = false + first.emit('disconnect') + const replacement = currentChild() + const secondPromise = subscribeViaWatcherProcess('/new', vi.fn(), {}) + + expect(replacement).not.toBe(first) + expect(replacement.sent.filter((message) => message.op === 'subscribe')).toEqual([ + expect.objectContaining({ dir: '/existing' }), + expect.objectContaining({ dir: '/new' }) + ]) + ackSubscribe(replacement, 0) + ackSubscribe(replacement, 1) + await secondPromise + }) + + it('spreads runtime roots across healthy children and contains a shard crash', async () => { + const firstInterruption = vi.fn() + const secondInterruption = vi.fn() + const firstPromise = subscribeViaRuntimeWatcherProcess( + '/repo-a', + vi.fn(), + {}, + { + onInterruption: firstInterruption + } + ) + const firstChild = currentChild() + ackSubscribe(firstChild) + await firstPromise + + const secondPromise = subscribeViaRuntimeWatcherProcess( + '/repo-b', + vi.fn(), + {}, + { + onInterruption: secondInterruption + } + ) + const secondChild = currentChild() + expect(secondChild).not.toBe(firstChild) + ackSubscribe(secondChild) + await secondPromise + expect(forkMock).toHaveBeenCalledTimes(2) + + firstChild.connected = false + firstChild.emit('exit', null, 'SIGSEGV') + + const replacement = currentChild() + expect(replacement).not.toBe(firstChild) + expect(replacement.sent.filter((message) => message.op === 'subscribe')).toHaveLength(1) + ackSubscribe(replacement) + expect(firstInterruption).toHaveBeenCalledTimes(1) + expect(secondInterruption).not.toHaveBeenCalled() + expect(secondChild.connected).toBe(true) + }) + it('completes an in-flight subscribe across a crash-respawn', async () => { - const promise = subscribeViaWatcherProcess('/repo', vi.fn(), {}) + const onInterruption = vi.fn() + const promise = subscribeViaWatcherProcess('/repo', vi.fn(), {}, { onInterruption }) const first = currentChild() expect(first.sent[0].op).toBe('subscribe') @@ -168,6 +280,190 @@ describe('subscribeViaWatcherProcess', () => { const second = currentChild() ackSubscribe(second) await expect(promise).resolves.toMatchObject({ unsubscribe: expect.any(Function) }) + expect(onInterruption).toHaveBeenCalledTimes(1) + }) + + it('cancels a queued crawl without restarting healthy roots', async () => { + const healthyInterruption = vi.fn() + const healthyPromise = subscribeViaWatcherProcess( + '/healthy', + vi.fn(), + {}, + { + onInterruption: healthyInterruption + } + ) + const child = currentChild() + ackSubscribe(child) + await healthyPromise + + const controller = new AbortController() + const pending = subscribeViaWatcherProcess( + '/queued', + vi.fn(), + {}, + { + signal: controller.signal + } + ) + const queuedId = child.sent.at(-1)?.id + controller.abort() + + await expect(pending).rejects.toThrow('aborted') + expect(child.kill).not.toHaveBeenCalled() + expect(child.sent.at(-1)).toEqual({ op: 'cancel-subscribe', id: queuedId }) + expect(healthyInterruption).not.toHaveBeenCalled() + }) + + it('cancels an active crawl and restores healthy roots when its owner aborts', async () => { + const healthyInterruption = vi.fn() + const healthyPromise = subscribeViaWatcherProcess( + '/healthy', + vi.fn(), + {}, + { + onInterruption: healthyInterruption + } + ) + const first = currentChild() + ackSubscribe(first) + await healthyPromise + + const controller = new AbortController() + const pending = subscribeViaWatcherProcess('/slow', vi.fn(), {}, { signal: controller.signal }) + first.emit('message', { op: 'subscribe-started', id: first.sent.at(-1)?.id }) + controller.abort() + + await expect(pending).rejects.toThrow('aborted') + expect(first.kill).toHaveBeenCalledTimes(1) + const replacement = currentChild() + expect(replacement).not.toBe(first) + expect(replacement.sent.filter((message) => message.op === 'subscribe')).toEqual([ + expect.objectContaining({ dir: '/healthy' }) + ]) + ackSubscribe(replacement) + expect(healthyInterruption).toHaveBeenCalledTimes(1) + }) + + it('bounds a pending crawl and restores healthy roots after setup timeout', async () => { + vi.useFakeTimers() + try { + const healthyInterruption = vi.fn() + const healthyPromise = subscribeViaWatcherProcess( + '/healthy', + vi.fn(), + {}, + { + onInterruption: healthyInterruption + } + ) + const first = currentChild() + ackSubscribe(first) + await healthyPromise + + const pending = subscribeViaWatcherProcess('/slow', vi.fn(), {}, { subscribeTimeoutMs: 100 }) + first.emit('message', { op: 'subscribe-started', id: first.sent.at(-1)?.id }) + const timedOut = expect(pending).rejects.toThrow('timed out') + await vi.advanceTimersByTimeAsync(100) + + await timedOut + const replacement = currentChild() + expect(replacement).not.toBe(first) + ackSubscribe(replacement) + expect(healthyInterruption).toHaveBeenCalledTimes(1) + } finally { + vi.useRealTimers() + } + }) + + it('ends a root whose crash resubscription crawl exceeds its deadline', async () => { + vi.useFakeTimers() + try { + const supervisor = createWatcherProcessSupervisor() + const onTerminalError = vi.fn() + const initial = supervisor.subscribe( + '/slow-recovery', + vi.fn(), + {}, + { + onTerminalError, + subscribeTimeoutMs: 100 + } + ) + const first = currentChild() + ackSubscribe(first) + await initial + + first.connected = false + first.emit('exit', null, 'SIGSEGV') + const replacement = currentChild() + replacement.emit('message', { + op: 'subscribe-started', + id: replacement.sent[0].id + }) + + await vi.advanceTimersByTimeAsync(100) + + expect(onTerminalError).toHaveBeenCalledWith( + expect.objectContaining({ + code: 'subscribe_timeout', + message: 'file watcher resubscription timed out after 100ms' + }) + ) + expect(replacement.kill).toHaveBeenCalledTimes(1) + expect(forkMock).toHaveBeenCalledTimes(2) + supervisor.dispose() + } finally { + vi.useRealTimers() + } + }) + + it('starts the setup timeout only when the child begins that crawl', async () => { + vi.useFakeTimers() + try { + const pending = subscribeViaWatcherProcess( + '/queued', + vi.fn(), + {}, + { + subscribeTimeoutMs: 100 + } + ) + const child = currentChild() + let settled = false + void pending.then( + () => { + settled = true + }, + () => { + settled = true + } + ) + + await vi.advanceTimersByTimeAsync(100) + expect(settled).toBe(false) + expect(child.kill).not.toHaveBeenCalled() + + child.emit('message', { op: 'subscribe-started', id: child.sent.at(-1)?.id }) + const timedOut = expect(pending).rejects.toThrow('timed out') + await vi.advanceTimersByTimeAsync(100) + await timedOut + } finally { + vi.useRealTimers() + } + }) + + it('rejects an in-flight and later subscribe when the supervisor is disposed', async () => { + const supervisor = createWatcherProcessSupervisor() + const pending = supervisor.subscribe('/repo', vi.fn(), {}) + const child = currentChild() + + supervisor.dispose() + + await expect(pending).rejects.toThrow('supervisor disposed') + await expect(supervisor.subscribe('/later', vi.fn(), {})).rejects.toThrow('supervisor disposed') + expect(child.kill).toHaveBeenCalledTimes(1) + expect(forkMock).toHaveBeenCalledTimes(1) }) it('disables watching after repeated crashes instead of fork-bombing', async () => { @@ -203,6 +499,10 @@ describe('subscribeViaWatcherProcess', () => { await expect(subscription.unsubscribe()).resolves.toBeUndefined() expect(first.kill).toHaveBeenCalledTimes(1) + expect(rmSyncMock).toHaveBeenCalledWith('/tmp/orca-watcher-canary-supervisor-test', { + recursive: true, + force: true + }) // 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. @@ -279,8 +579,101 @@ describe('subscribeViaWatcherProcess', () => { ignore: ['**/.git'] }) expect(forkMock).not.toHaveBeenCalled() - expect(parcelSubscribeMock).toHaveBeenCalledWith('/repo', callback, { ignore: ['**/.git'] }) + expect(parcelSubscribeMock).toHaveBeenCalledWith('/repo', expect.any(Function), { + ignore: ['**/.git'] + }) await subscription.unsubscribe() expect(unsubscribe).toHaveBeenCalled() }) + + it('rejects a pre-aborted in-process subscribe before loading Parcel', async () => { + vi.stubEnv('VITEST', 'true') + const controller = new AbortController() + controller.abort() + + await expect( + subscribeViaWatcherProcess('/repo', vi.fn(), {}, { signal: controller.signal }) + ).rejects.toThrow('aborted') + expect(parcelSubscribeMock).not.toHaveBeenCalled() + }) + + it('releases a late in-process subscription after pending setup is aborted', async () => { + vi.stubEnv('VITEST', 'true') + const unsubscribe = vi.fn().mockResolvedValue(undefined) + let resolveSubscription!: (subscription: { unsubscribe: typeof unsubscribe }) => void + parcelSubscribeMock.mockReturnValue( + new Promise((resolve) => { + resolveSubscription = resolve + }) + ) + const controller = new AbortController() + const pending = subscribeViaWatcherProcess('/repo', vi.fn(), {}, { signal: controller.signal }) + + await vi.waitFor(() => expect(parcelSubscribeMock).toHaveBeenCalledTimes(1)) + controller.abort() + await expect(pending).rejects.toThrow('aborted') + resolveSubscription({ unsubscribe }) + + await vi.waitFor(() => expect(unsubscribe).toHaveBeenCalledTimes(1)) + }) + + it('fails closed when the isolated watcher entry is absent outside tests', async () => { + existsSyncMock.mockReturnValue(false) + + await expect(subscribeViaWatcherProcess('/repo', vi.fn(), {})).rejects.toThrow( + 'watcher process entry is missing' + ) + expect(forkMock).not.toHaveBeenCalled() + expect(parcelSubscribeMock).not.toHaveBeenCalled() + }) + + it('starts without a canary when the diagnostic temp directory is unavailable', async () => { + mkdtempSyncMock.mockImplementationOnce(() => { + throw new Error('read-only temp directory') + }) + + const pending = subscribeViaWatcherProcess('/repo', vi.fn(), {}) + const child = currentChild() + const forkOptions = forkMock.mock.calls.at(-1)?.[2] as { env?: NodeJS.ProcessEnv } | undefined + expect(forkOptions?.env?.ORCA_WATCHER_CANARY_DIR).toBeUndefined() + ackSubscribe(child) + await expect(pending).resolves.toMatchObject({ unsubscribe: expect.any(Function) }) + }) + + it('keeps independently-created supervisors in separate crash-fuse domains', async () => { + const firstSupervisor = createWatcherProcessSupervisor() + const secondSupervisor = createWatcherProcessSupervisor() + const firstCallback = vi.fn() + const secondCallback = vi.fn() + + const firstPromise = firstSupervisor.subscribe('/faulting', firstCallback, {}) + const firstChild = currentChild() + ackSubscribe(firstChild) + await firstPromise + + const secondPromise = secondSupervisor.subscribe('/healthy', secondCallback, {}) + const secondChild = currentChild() + ackSubscribe(secondChild) + await secondPromise + + let faultingChild = firstChild + for (let crash = 0; crash < 2; crash++) { + faultingChild.connected = false + faultingChild.emit('exit', null, 'SIGSEGV') + faultingChild = currentChild() + ackSubscribe(faultingChild) + } + faultingChild.connected = false + faultingChild.emit('exit', null, 'SIGSEGV') + + expect(firstCallback).toHaveBeenCalledWith( + expect.objectContaining({ message: expect.stringContaining('crashed repeatedly') }), + [] + ) + expect(secondCallback).not.toHaveBeenCalled() + expect(secondChild.connected).toBe(true) + + firstSupervisor.dispose() + secondSupervisor.dispose() + }) }) diff --git a/src/main/ipc/parcel-watcher-process.ts b/src/main/ipc/parcel-watcher-process.ts index 72e2ccf58ce..2d6b45ee97f 100644 --- a/src/main/ipc/parcel-watcher-process.ts +++ b/src/main/ipc/parcel-watcher-process.ts @@ -1,329 +1,75 @@ -// Host side of the out-of-process @parcel/watcher (see -// parcel-watcher-process-entry.ts). Why: native watcher teardown races -// fail-fast the hosting process (issue #7547), so local subscriptions live in -// a disposable forked child; when it crashes the host respawns it and -// resubscribes every live root, and callers are told via onInterruption so -// they can refresh state that changed during the gap. -import { fork, type ChildProcess } from 'node:child_process' -import { existsSync } from 'node:fs' -import type * as ParcelWatcher from '@parcel/watcher' -import { getWatcherProcessEntryPath } from './parcel-watcher-entry-path' -import type { - HostToWatcherMessage, +// Host API for crash-isolated @parcel/watcher children. Desktop uses one +// supervisor; runtime roots use a bounded pool with independent crash fuses. +import { + WatcherProcessSupervisor, + type WatcherProcessCallback, + type WatcherProcessHooks, + type WatcherProcessSubscription +} from './parcel-watcher-process-supervisor' +import type { WatcherProcessSubscribeOptions } from './parcel-watcher-process-protocol' +import { RuntimeWatcherProcessPool } from './runtime-watcher-process-pool' + +export type { + WatcherProcessCallback, + WatcherProcessHooks, + WatcherProcessSubscription +} from './parcel-watcher-process-supervisor' +export type { + WatcherProcessDeliveryOptions, WatcherProcessEvent, - WatcherProcessSubscribeOptions, - WatcherToHostMessage -} from './parcel-watcher-process-entry' + WatcherProcessSubscribeOptions +} from './parcel-watcher-process-protocol' -export type { WatcherProcessEvent, WatcherProcessSubscribeOptions } +const sharedWatcherProcessSupervisor = new WatcherProcessSupervisor() +// Why: healthy and quarantine pools each cap at four children, containing a +// failed shard without scaling process count with its number of roots. +const runtimeWatcherProcessPool = new RuntimeWatcherProcessPool() -export type WatcherProcessCallback = (err: Error | null, events: WatcherProcessEvent[]) => void -export type WatcherProcessSubscription = { unsubscribe(): Promise } - -// Why: a crash loop (e.g. a root that faults the native watcher on every -// resubscribe) must degrade to "file watching disabled" rather than fork-bomb. -const CRASH_WINDOW_MS = 30_000 -const MAX_CRASHES_PER_WINDOW = 3 - -type SubscriptionRecord = { - id: number - dir: string - opts: WatcherProcessSubscribeOptions - callback: WatcherProcessCallback - onInterruption?: () => void - pendingSubscribe?: { resolve: () => void; reject: (err: Error) => void } +export function createWatcherProcessSupervisor(): WatcherProcessSupervisor { + return new WatcherProcessSupervisor() } -let child: ChildProcess | null = null -let nextSubscriptionId = 1 -let crashTimes: number[] = [] -let shutdownRequested = false -let loggedInProcessFallback = false -const records = new Map() -const pendingUnsubscribes = new Map void>() - -function shouldRunInProcess(entryPath: string): boolean { - // Why: vitest suites mock '@parcel/watcher' at the module level; a forked - // child would bypass those mocks (and could execute a stale build output), - // so tests keep the historical in-process path. - if (process.env.VITEST) { - return true - } - if (existsSync(entryPath)) { - return false - } - if (!loggedInProcessFallback) { - loggedInProcessFallback = true - console.warn( - `[parcel-watcher-process] entry not found at ${entryPath}; using in-process watcher (no crash isolation)` - ) - } - return true -} - -async function subscribeInProcess( - dir: string, - callback: WatcherProcessCallback, - opts: WatcherProcessSubscribeOptions -): Promise { - const watcher = await import('@parcel/watcher') - const subscription = await watcher.subscribe( - dir, - callback as ParcelWatcher.SubscribeCallback, - opts as ParcelWatcher.Options - ) - return { unsubscribe: () => subscription.unsubscribe() } -} - -function inCrashCooldown(): boolean { - const now = Date.now() - crashTimes = crashTimes.filter((time) => now - time < CRASH_WINDOW_MS) - return crashTimes.length >= MAX_CRASHES_PER_WINDOW -} - -function sendToChild(proc: ChildProcess, message: HostToWatcherMessage): void { - try { - proc.send(message) - } catch { - // Channel already closed — the child's exit handler recovers. - } -} - -function ensureWatcherProcess(): ChildProcess | null { - if (shutdownRequested) { - return null - } - if (child?.connected) { - return child - } - if (inCrashCooldown()) { - return null - } - let proc: ChildProcess - try { - proc = fork(getWatcherProcessEntryPath(), [], { - stdio: ['ignore', 'ignore', 'pipe', 'ipc'], - env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' }, - ...(process.platform === 'win32' ? { windowsHide: true } : {}) - }) - } catch (err) { - console.error('[parcel-watcher-process] failed to fork watcher process:', err) - return null - } - proc.stderr?.on('data', (chunk: Buffer) => { - console.error('[parcel-watcher-process]', String(chunk).trimEnd()) - }) - proc.on('message', (message) => { - if (child === proc) { - handleChildMessage(message as WatcherToHostMessage) - } - }) - proc.on('error', () => handleChildGone(proc)) - proc.on('exit', (code, signal) => { - if (child === proc && (code !== 0 || signal)) { - console.error( - `[parcel-watcher-process] watcher process exited (code=${code}, signal=${signal})` - ) - } - handleChildGone(proc) - }) - child = proc - return proc -} - -function handleChildMessage(message: WatcherToHostMessage): void { - if (!message || typeof message !== 'object') { - return - } - if (message.op === 'unsubscribed') { - const resolve = pendingUnsubscribes.get(message.id) - pendingUnsubscribes.delete(message.id) - resolve?.() - return - } - const record = records.get(message.id) - if (!record) { - return - } - if (message.op === 'subscribed') { - const pending = record.pendingSubscribe - record.pendingSubscribe = undefined - if (pending) { - pending.resolve() - } else { - // Reached after a crash-respawn resubscribe: events emitted while the - // watcher process was down are lost, so let the caller refresh. - record.onInterruption?.() - } - return - } - if (message.op === 'subscribe-failed') { - records.delete(message.id) - const pending = record.pendingSubscribe - record.pendingSubscribe = undefined - if (pending) { - pending.reject(new Error(message.message)) - } 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') { - record.callback(null, message.events) - return - } - if (message.op === 'watch-error') { - record.callback(new Error(message.message), []) - } -} - -function failAllSubscriptions(err: Error): void { - for (const record of records.values()) { - const pending = record.pendingSubscribe - record.pendingSubscribe = undefined - if (pending) { - pending.reject(err) - } else { - record.callback(err, []) - } - } - records.clear() -} - -function handleChildGone(proc: ChildProcess): void { - if (child !== proc) { - return - } - child = null - // Why: process death released every native handle, so a pending unsubscribe - // is complete by definition — resolve rather than hang worktree deletion. - for (const resolve of pendingUnsubscribes.values()) { - resolve() - } - pendingUnsubscribes.clear() - if (shutdownRequested || records.size === 0) { - return - } - crashTimes.push(Date.now()) - const replacement = ensureWatcherProcess() - if (!replacement) { - console.error( - '[parcel-watcher-process] watcher process crashed repeatedly; disabling file watching' - ) - failAllSubscriptions(new Error('file watcher process crashed repeatedly')) - return - } - console.error( - `[parcel-watcher-process] watcher process crashed; resubscribing ${records.size} root(s)` - ) - for (const record of records.values()) { - sendToChild(replacement, { - op: 'subscribe', - id: record.id, - dir: record.dir, - opts: record.opts - }) - } -} - -// 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 => { - if (!records.delete(record.id)) { - return Promise.resolve() - } - const proc = child - 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 }) - }) - } - } -} - -/** Subscribe to a directory tree via the isolated watcher process. Falls back - * to an in-process @parcel/watcher subscription when the forked entry is not - * available (tests, unbuilt trees). `onInterruption` fires after the watcher - * process crashed and this subscription was transparently re-established — - * events in the gap were lost, so callers should refresh. */ export function subscribeViaWatcherProcess( dir: string, callback: WatcherProcessCallback, opts: WatcherProcessSubscribeOptions, - onInterruption?: () => void + hooks: WatcherProcessHooks = {} ): Promise { - if (shouldRunInProcess(getWatcherProcessEntryPath())) { - return subscribeInProcess(dir, callback, opts) - } - const record: SubscriptionRecord = { - id: nextSubscriptionId++, - dir, - opts, - callback, - onInterruption - } - return new Promise((resolve, reject) => { - const proc = ensureWatcherProcess() - if (!proc) { - reject(new Error('file watcher process unavailable')) - return - } - records.set(record.id, record) - record.pendingSubscribe = { - resolve: () => resolve(makeSubscription(record)), - reject - } - sendToChild(proc, { op: 'subscribe', id: record.id, dir, opts }) - }) + return sharedWatcherProcessSupervisor.subscribe(dir, callback, opts, hooks) } -/** Kill the watcher process at app shutdown. Process death releases native - * handles without running watcher.node's crash-prone async teardown. */ +export function subscribeViaRuntimeWatcherProcess( + dir: string, + callback: WatcherProcessCallback, + opts: WatcherProcessSubscribeOptions, + hooks: WatcherProcessHooks = {} +): Promise { + return runtimeWatcherProcessPool.subscribe(dir, callback, opts, hooks) +} + +/** Kill shared desktop and runtime watcher children at app/runtime shutdown. */ export function disposeWatcherProcess(): void { - shutdownRequested = true - const proc = child - child = null - for (const resolve of pendingUnsubscribes.values()) { - resolve() + // Why: Vitest reuses this module singleton after closeAllWatchers(), while + // production shutdown must remain final and reject any later subscription. + if (process.env.VITEST) { + sharedWatcherProcessSupervisor.resetForTest() + runtimeWatcherProcessPool.resetForTest() + } else { + sharedWatcherProcessSupervisor.dispose() + // Why: the runtime pool owns independent children that otherwise outlive a + // shutdown sequence that does not immediately exit the main process. + runtimeWatcherProcessPool.dispose() } - pendingUnsubscribes.clear() - records.clear() - proc?.kill() } export function resetWatcherProcessForTest(): void { - disposeWatcherProcess() - shutdownRequested = false - crashTimes = [] - loggedInProcessFallback = false - nextSubscriptionId = 1 + sharedWatcherProcessSupervisor.resetForTest() +} + +export function resetRuntimeWatcherProcessForTest(): void { + runtimeWatcherProcessPool.resetForTest() +} + +export function forgetRuntimeWatcherProcessRoot(rootPath: string): void { + runtimeWatcherProcessPool.forgetRoot(rootPath) } diff --git a/src/main/ipc/parcel-watcher-supervisor-subscribe.ts b/src/main/ipc/parcel-watcher-supervisor-subscribe.ts new file mode 100644 index 00000000000..3f0fd8ad415 --- /dev/null +++ b/src/main/ipc/parcel-watcher-supervisor-subscribe.ts @@ -0,0 +1,127 @@ +import type { ChildProcess } from 'node:child_process' +import { existsSync } from 'node:fs' +import { getWatcherProcessEntryPath } from './parcel-watcher-entry-path' +import { createHostWatcherSubscription } from './parcel-watcher-host-subscriptions' +import { subscribeWithInProcessWatcher } from './parcel-watcher-in-process-fallback' +import { installPendingSubscribeControls } from './parcel-watcher-pending-subscribe' +import { WatcherProcessFailure } from './parcel-watcher-process-failure' +import type { + HostToWatcherMessage, + WatcherProcessSubscribeOptions +} from './parcel-watcher-process-protocol' +import type { + WatcherProcessCallback, + WatcherProcessHooks, + WatcherProcessSubscription, + WatcherProcessSubscriptionRecord +} from './parcel-watcher-process-subscription' + +type WatcherSupervisorSubscribeOptions = { + dir: string + callback: WatcherProcessCallback + opts: WatcherProcessSubscribeOptions + hooks: WatcherProcessHooks + shutdownRequested: boolean + allocateId: () => number + records: Map + pendingUnsubscribes: Map void> + ensureWatcherProcess: (entryPath: string) => ChildProcess | null + getChild: () => ChildProcess | null + killWatcherChildIfIdle: () => void + sendSubscribe: (child: ChildProcess, record: WatcherProcessSubscriptionRecord) => void + sendToChild: (child: ChildProcess, message: HostToWatcherMessage) => void + cancelPendingSubscribe: ( + record: WatcherProcessSubscriptionRecord, + error: WatcherProcessFailure + ) => void +} + +export function subscribeThroughWatcherSupervisor({ + dir, + callback, + opts, + hooks, + shutdownRequested, + allocateId, + records, + pendingUnsubscribes, + ensureWatcherProcess, + getChild, + killWatcherChildIfIdle, + sendSubscribe, + sendToChild, + cancelPendingSubscribe +}: WatcherSupervisorSubscribeOptions): Promise { + if (shutdownRequested) { + return Promise.reject( + new WatcherProcessFailure( + 'file watcher supervisor disposed', + 'supervisor', + 'supervisor_disposed' + ) + ) + } + if (hooks.signal?.aborted) { + return Promise.reject( + new WatcherProcessFailure( + 'file watcher subscription aborted', + 'subscription', + 'subscribe_aborted' + ) + ) + } + // Why: under Vitest we cannot fork a real watcher child, so exercise the + // subscription path in-process (against mocked @parcel/watcher) instead. + if (process.env.VITEST) { + return subscribeWithInProcessWatcher(dir, callback, opts, hooks) + } + const entryPath = getWatcherProcessEntryPath() + if (!existsSync(entryPath)) { + return Promise.reject( + new WatcherProcessFailure( + `watcher process entry is missing: ${entryPath}`, + 'supervisor', + 'entry_missing' + ) + ) + } + const record: WatcherProcessSubscriptionRecord = { + id: allocateId(), + dir, + opts, + callback, + hooks, + interrupted: false, + crawlStarted: false + } + return new Promise((resolve, reject) => { + const child = ensureWatcherProcess(entryPath) + if (!child) { + reject( + new WatcherProcessFailure( + 'file watcher process unavailable', + 'supervisor', + 'process_unavailable' + ) + ) + return + } + records.set(record.id, record) + record.pendingSubscribe = { + resolve: () => + resolve( + createHostWatcherSubscription({ + record, + records, + pendingUnsubscribes, + getChild, + killWatcherChildIfIdle, + sendToChild + }) + ), + reject + } + installPendingSubscribeControls(record, (error) => cancelPendingSubscribe(record, error)) + sendSubscribe(child, record) + }) +} diff --git a/src/main/ipc/runtime-watcher-process-pool.test.ts b/src/main/ipc/runtime-watcher-process-pool.test.ts new file mode 100644 index 00000000000..e43112e4ef4 --- /dev/null +++ b/src/main/ipc/runtime-watcher-process-pool.test.ts @@ -0,0 +1,273 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { WatcherProcessFailure } from './parcel-watcher-process-failure' +import type { + WatcherProcessCallback, + WatcherProcessHooks, + WatcherProcessSubscription +} from './parcel-watcher-process-subscription' +import { RuntimeWatcherProcessPool } from './runtime-watcher-process-pool' + +type InstalledSubscription = { + dir: string + hooks: WatcherProcessHooks + unsubscribe: ReturnType Promise>> +} + +class FakeSupervisor { + readonly subscriptions: InstalledSubscription[] = [] + readonly dispose = vi.fn() + subscribeError?: Error + + async subscribe( + dir: string, + _callback: WatcherProcessCallback, + _opts: object, + hooks: WatcherProcessHooks + ): Promise { + if (this.subscribeError) { + throw this.subscribeError + } + const unsubscribe = vi.fn(async () => undefined) + this.subscriptions.push({ dir, hooks, unsubscribe }) + return { unsubscribe } + } +} + +describe('RuntimeWatcherProcessPool', () => { + let supervisors: FakeSupervisor[] + let pool: RuntimeWatcherProcessPool + + beforeEach(() => { + supervisors = [] + pool = new RuntimeWatcherProcessPool({ + maxSharedSupervisors: 4, + createSupervisor: () => { + const supervisor = new FakeSupervisor() + supervisors.push(supervisor) + return supervisor + } + }) + }) + + it('spreads roots across a bounded healthy pool before sharing children', async () => { + for (const dir of ['/a', '/b', '/c', '/d', '/e']) { + await pool.subscribe(dir, vi.fn(), {}, {}) + } + + expect(supervisors).toHaveLength(4) + expect(supervisors.map((supervisor) => supervisor.subscriptions.map(({ dir }) => dir))).toEqual( + [['/a', '/e'], ['/b'], ['/c'], ['/d']] + ) + }) + + it('keeps a newer same-root lease assigned while an older lease tears down', async () => { + const first = await pool.subscribe('/same', vi.fn(), {}, {}) + await pool.subscribe('/same', vi.fn(), {}, {}) + + await first.unsubscribe() + await pool.subscribe('/same', vi.fn(), {}, {}) + + expect(supervisors).toHaveLength(1) + expect(supervisors[0].subscriptions.map(({ dir }) => dir)).toEqual(['/same', '/same', '/same']) + }) + + it('isolates every root from a failed shard when callers recover', async () => { + pool = new RuntimeWatcherProcessPool({ + maxSharedSupervisors: 1, + createSupervisor: () => { + const supervisor = new FakeSupervisor() + supervisors.push(supervisor) + return supervisor + } + }) + const firstError = vi.fn() + const secondError = vi.fn() + await pool.subscribe('/a', vi.fn(), {}, { onTerminalError: firstError }) + await pool.subscribe('/b', vi.fn(), {}, { onTerminalError: secondError }) + const failedShard = supervisors[0] + const failure = new WatcherProcessFailure( + 'file watcher process crashed repeatedly', + 'supervisor', + 'supervisor_crash_fuse' + ) + + for (const installed of failedShard.subscriptions) { + installed.hooks.onTerminalError?.(failure) + } + await pool.subscribe('/a', vi.fn(), {}, {}) + await pool.subscribe('/b', vi.fn(), {}, {}) + + expect(firstError).toHaveBeenCalledWith(failure) + expect(secondError).toHaveBeenCalledWith(failure) + expect(supervisors).toHaveLength(3) + expect(supervisors[1].subscriptions.map(({ dir }) => dir)).toEqual(['/a']) + expect(supervisors[2].subscriptions.map(({ dir }) => dir)).toEqual(['/b']) + }) + + it('bounds quarantine supervisors when a fused shard contains many roots', async () => { + pool = new RuntimeWatcherProcessPool({ + maxSharedSupervisors: 1, + createSupervisor: () => { + const supervisor = new FakeSupervisor() + supervisors.push(supervisor) + return supervisor + } + }) + const roots = Array.from({ length: 12 }, (_, index) => `/root-${index}`) + for (const root of roots) { + await pool.subscribe(root, vi.fn(), {}, {}) + } + const failedShard = supervisors[0] + const failure = new WatcherProcessFailure( + 'file watcher process crashed repeatedly', + 'supervisor', + 'supervisor_crash_fuse' + ) + + for (const installed of failedShard.subscriptions) { + installed.hooks.onTerminalError?.(failure) + } + for (const root of roots) { + await pool.subscribe(root, vi.fn(), {}, {}) + } + + expect(supervisors).toHaveLength(5) + expect(supervisors.slice(1).map(({ subscriptions }) => subscriptions.length)).toEqual([ + 3, 3, 3, 3 + ]) + }) + + it('moves an explicitly timed-out root into quarantine for its retry', async () => { + const timeout = new WatcherProcessFailure( + 'file watcher subscription timed out', + 'subscription', + 'subscribe_timeout' + ) + pool = new RuntimeWatcherProcessPool({ + maxSharedSupervisors: 1, + createSupervisor: () => { + const supervisor = new FakeSupervisor() + if (supervisors.length === 0) { + supervisor.subscribeError = timeout + } + supervisors.push(supervisor) + return supervisor + } + }) + + await expect(pool.subscribe('/slow', vi.fn(), {}, {})).rejects.toBe(timeout) + await expect(pool.subscribe('/slow', vi.fn(), {}, {})).resolves.toBeDefined() + + expect(supervisors).toHaveLength(2) + expect(supervisors[1].subscriptions.map(({ dir }) => dir)).toEqual(['/slow']) + }) + + it('moves a live root into quarantine when crash resubscription times out', async () => { + const timeout = new WatcherProcessFailure( + 'file watcher resubscription timed out', + 'subscription', + 'subscribe_timeout' + ) + const onTerminalError = vi.fn() + await pool.subscribe('/slow-recovery', vi.fn(), {}, { onTerminalError }) + + supervisors[0].subscriptions[0].hooks.onTerminalError?.(timeout) + await pool.subscribe('/slow-recovery', vi.fn(), {}, {}) + + expect(onTerminalError).toHaveBeenCalledWith(timeout) + expect(supervisors).toHaveLength(2) + expect(supervisors[1].subscriptions.map(({ dir }) => dir)).toEqual(['/slow-recovery']) + }) + + it('keeps healthy shard assignments after a root-specific failure', async () => { + pool = new RuntimeWatcherProcessPool({ + maxSharedSupervisors: 1, + createSupervisor: () => { + const supervisor = new FakeSupervisor() + supervisors.push(supervisor) + return supervisor + } + }) + await pool.subscribe('/healthy', vi.fn(), {}, {}) + const onTerminalError = vi.fn() + await pool.subscribe('/gone', vi.fn(), {}, { onTerminalError }) + const shared = supervisors[0] + const failed = shared.subscriptions.find(({ dir }) => dir === '/gone') + const failure = new WatcherProcessFailure( + 'root unavailable', + 'subscription', + 'subscribe_failed' + ) + + failed?.hooks.onTerminalError?.(failure) + await pool.subscribe('/gone', vi.fn(), {}, {}) + + expect(onTerminalError).toHaveBeenCalledWith(failure) + expect(supervisors).toHaveLength(1) + expect(shared.subscriptions.map(({ dir }) => dir)).toEqual(['/healthy', '/gone', '/gone']) + expect(shared.dispose).not.toHaveBeenCalled() + }) + + it('stops replacing a root after its bounded quarantine shard also fuses', async () => { + pool = new RuntimeWatcherProcessPool({ + maxSharedSupervisors: 1, + createSupervisor: () => { + const supervisor = new FakeSupervisor() + supervisors.push(supervisor) + return supervisor + } + }) + const failure = new WatcherProcessFailure( + 'file watcher process crashed repeatedly', + 'supervisor', + 'supervisor_crash_fuse' + ) + await pool.subscribe('/unstable', vi.fn(), {}, {}) + supervisors[0].subscriptions[0].hooks.onTerminalError?.(failure) + await pool.subscribe('/unstable', vi.fn(), {}, {}) + supervisors[1].subscriptions[0].hooks.onTerminalError?.(failure) + + await expect(pool.subscribe('/unstable', vi.fn(), {}, {})).rejects.toThrow( + 'failed again in quarantine' + ) + expect(supervisors).toHaveLength(2) + }) + + it('clears fused quarantine history when forgetRoot runs without an assignment', async () => { + pool = new RuntimeWatcherProcessPool({ + maxSharedSupervisors: 1, + createSupervisor: () => { + const supervisor = new FakeSupervisor() + supervisors.push(supervisor) + return supervisor + } + }) + const failure = new WatcherProcessFailure( + 'file watcher process crashed repeatedly', + 'supervisor', + 'supervisor_crash_fuse' + ) + await pool.subscribe('/unstable', vi.fn(), {}, {}) + supervisors[0].subscriptions[0].hooks.onTerminalError?.(failure) + await pool.subscribe('/unstable', vi.fn(), {}, {}) + supervisors[1].subscriptions[0].hooks.onTerminalError?.(failure) + + await expect(pool.subscribe('/unstable', vi.fn(), {}, {})).rejects.toThrow( + 'failed again in quarantine' + ) + + pool.forgetRoot('/unstable') + + await expect(pool.subscribe('/unstable', vi.fn(), {}, {})).resolves.toBeDefined() + expect(supervisors).toHaveLength(3) + }) + + it('disposes every supervisor and clears isolation on reset', async () => { + await pool.subscribe('/a', vi.fn(), {}, {}) + await pool.subscribe('/b', vi.fn(), {}, {}) + + pool.resetForTest() + + expect(supervisors.every((supervisor) => supervisor.dispose.mock.calls.length === 1)).toBe(true) + }) +}) diff --git a/src/main/ipc/runtime-watcher-process-pool.ts b/src/main/ipc/runtime-watcher-process-pool.ts new file mode 100644 index 00000000000..b5a2bf45d44 --- /dev/null +++ b/src/main/ipc/runtime-watcher-process-pool.ts @@ -0,0 +1,251 @@ +import { isWatcherProcessFailure, WatcherProcessFailure } from './parcel-watcher-process-failure' +import { WatcherProcessSupervisor } from './parcel-watcher-process-supervisor' +import type { WatcherProcessSubscribeOptions } from './parcel-watcher-process-protocol' +import type { + WatcherProcessCallback, + WatcherProcessHooks, + WatcherProcessSubscription +} from './parcel-watcher-process-subscription' + +const DEFAULT_MAX_SHARED_SUPERVISORS = 4 +const DEFAULT_MAX_QUARANTINE_SUPERVISORS = 4 + +type WatcherSupervisor = Pick + +type RuntimeWatcherPoolSlot = { + supervisor: WatcherSupervisor + roots: Set + isolated: boolean + retired: boolean + disposed: boolean +} + +type RuntimeWatcherPoolAssignment = { + slot: RuntimeWatcherPoolSlot + leases: number +} + +export type RuntimeWatcherProcessPoolOptions = { + maxSharedSupervisors?: number + maxQuarantineSupervisors?: number + createSupervisor?: () => WatcherSupervisor +} + +/** + * Share a bounded set of watcher children while healthy, then move roots from + * a failed shard into a separate bounded quarantine pool. + */ +export class RuntimeWatcherProcessPool { + private readonly maxSharedSupervisors: number + private readonly maxQuarantineSupervisors: number + private readonly createSupervisor: () => WatcherSupervisor + private readonly activeSlots = new Set() + private readonly allSlots = new Set() + private readonly assignments = new Map() + private readonly isolatedRoots = new Set() + private readonly failedQuarantineRoots = new Set() + + constructor(options: RuntimeWatcherProcessPoolOptions = {}) { + this.maxSharedSupervisors = Math.max( + 1, + options.maxSharedSupervisors ?? DEFAULT_MAX_SHARED_SUPERVISORS + ) + this.maxQuarantineSupervisors = Math.max( + 1, + options.maxQuarantineSupervisors ?? DEFAULT_MAX_QUARANTINE_SUPERVISORS + ) + this.createSupervisor = options.createSupervisor ?? (() => new WatcherProcessSupervisor()) + } + + async subscribe( + dir: string, + callback: WatcherProcessCallback, + opts: WatcherProcessSubscribeOptions, + hooks: WatcherProcessHooks = {} + ): Promise { + const assignment = this.assignmentForRoot(dir) + const { slot } = assignment + let assignmentReleased = false + const releaseAssignment = (): void => { + if (assignmentReleased) { + return + } + assignmentReleased = true + this.releaseRoot(assignment, dir) + } + const onTerminalError = (error: Error): void => { + if (isWatcherProcessFailure(error) && error.scope === 'supervisor') { + this.retireSlot(slot) + } else { + releaseAssignment() + if (isWatcherProcessFailure(error) && error.code === 'subscribe_timeout') { + this.isolatedRoots.add(dir) + } + } + hooks.onTerminalError?.(error) + } + let subscription: WatcherProcessSubscription + try { + subscription = await slot.supervisor.subscribe(dir, callback, opts, { + ...hooks, + onTerminalError + }) + } catch (error) { + if (isWatcherProcessFailure(error) && error.scope === 'supervisor') { + this.retireSlot(slot) + } else { + releaseAssignment() + if (isWatcherProcessFailure(error) && error.code === 'subscribe_timeout') { + this.isolatedRoots.add(dir) + } + } + throw error + } + + let unsubscribePromise: Promise | undefined + return { + unsubscribe: (): Promise => { + unsubscribePromise ??= subscription.unsubscribe().finally(() => { + releaseAssignment() + }) + return unsubscribePromise + } + } + } + + /** Kill every pooled watcher child (production shutdown or test reset). */ + dispose(): void { + // disposeSlot deletes the current slot from allSlots; deleting the + // in-progress element during Set iteration is safe, so no snapshot needed. + for (const slot of this.allSlots) { + this.disposeSlot(slot) + } + this.activeSlots.clear() + this.allSlots.clear() + this.assignments.clear() + this.isolatedRoots.clear() + this.failedQuarantineRoots.clear() + } + + resetForTest(): void { + this.dispose() + } + + forgetRoot(dir: string): void { + // Physical subscriptions release their assignment through unsubscribe or + // terminal callbacks. This clears fault history after setup gives up. + if (!this.assignments.has(dir)) { + this.isolatedRoots.delete(dir) + this.failedQuarantineRoots.delete(dir) + } + } + + private assignmentForRoot(dir: string): RuntimeWatcherPoolAssignment { + if (this.failedQuarantineRoots.has(dir)) { + throw new WatcherProcessFailure( + 'file watcher process failed again in quarantine', + 'supervisor', + 'supervisor_crash_fuse' + ) + } + const assigned = this.assignments.get(dir) + if (assigned && !assigned.slot.retired) { + assigned.leases++ + return assigned + } + const slot = this.isolatedRoots.has(dir) ? this.quarantineSlot() : this.sharedSlot() + const assignment = { slot, leases: 1 } + slot.roots.add(dir) + this.assignments.set(dir, assignment) + return assignment + } + + private sharedSlot(): RuntimeWatcherPoolSlot { + const sharedSlots = Array.from(this.activeSlots).filter( + (slot) => !slot.isolated && !slot.retired + ) + if (sharedSlots.length < this.maxSharedSupervisors) { + return this.createSlot(false) + } + return sharedSlots.reduce((leastLoaded, candidate) => + candidate.roots.size < leastLoaded.roots.size ? candidate : leastLoaded + ) + } + + private quarantineSlot(): RuntimeWatcherPoolSlot { + const quarantineSlots = Array.from(this.activeSlots).filter( + (slot) => slot.isolated && !slot.retired + ) + if (quarantineSlots.length < this.maxQuarantineSupervisors) { + return this.createSlot(true) + } + return quarantineSlots.reduce((leastLoaded, candidate) => + candidate.roots.size < leastLoaded.roots.size ? candidate : leastLoaded + ) + } + + private createSlot(isolated: boolean): RuntimeWatcherPoolSlot { + const slot: RuntimeWatcherPoolSlot = { + supervisor: this.createSupervisor(), + roots: new Set(), + isolated, + retired: false, + disposed: false + } + this.activeSlots.add(slot) + this.allSlots.add(slot) + return slot + } + + private retireSlot(slot: RuntimeWatcherPoolSlot): void { + if (slot.retired) { + return + } + slot.retired = true + this.activeSlots.delete(slot) + for (const root of slot.roots) { + if (this.assignments.get(root)?.slot === slot) { + this.assignments.delete(root) + } + if (slot.isolated) { + // Why: one bounded quarantine attempt is the recovery budget for a + // watch lifetime; repeated fused replacements would recreate churn. + this.isolatedRoots.delete(root) + this.failedQuarantineRoots.add(root) + } else { + this.isolatedRoots.add(root) + } + } + slot.roots.clear() + // Why: failAllSubscriptions is still iterating callbacks; defer disposal + // so every logical root receives the supervisor failure first. + queueMicrotask(() => this.disposeSlot(slot)) + } + + private releaseRoot(assignment: RuntimeWatcherPoolAssignment, dir: string): void { + if (this.assignments.get(dir) !== assignment) { + return + } + assignment.leases-- + if (assignment.leases > 0) { + return + } + const { slot } = assignment + this.assignments.delete(dir) + slot.roots.delete(dir) + this.isolatedRoots.delete(dir) + if (slot.isolated && slot.roots.size === 0 && !slot.retired) { + this.activeSlots.delete(slot) + this.disposeSlot(slot) + } + } + + private disposeSlot(slot: RuntimeWatcherPoolSlot): void { + if (slot.disposed) { + return + } + slot.disposed = true + slot.supervisor.dispose() + this.allSlots.delete(slot) + } +} diff --git a/src/main/providers/ssh-filesystem-provider-watch.ts b/src/main/providers/ssh-filesystem-provider-watch.ts index 7beea078ac3..02b87f3d005 100644 --- a/src/main/providers/ssh-filesystem-provider-watch.ts +++ b/src/main/providers/ssh-filesystem-provider-watch.ts @@ -4,6 +4,51 @@ import type { FsChangeEvent } from '../../shared/types' export type WatchRegistration = { callbacks: Set<(events: FsChangeEvent[]) => void> setupPromise: Promise + setupAbortController: AbortController + ready: boolean + stopping: boolean + unwatchSent: boolean +} + +function createWatchAbortError(): Error { + const error = new Error('Request "fs.watch" was cancelled') as Error & { name: string } + error.name = 'AbortError' + return error +} + +async function awaitSetupWithOptionalAbort( + setupPromise: Promise, + signal?: AbortSignal +): Promise { + if (!signal) { + await setupPromise + return + } + if (signal.aborted) { + throw createWatchAbortError() + } + await new Promise((resolve, reject) => { + const onAbort = (): void => { + cleanup() + reject(createWatchAbortError()) + } + const onSettled = (error?: unknown): void => { + cleanup() + if (error === undefined) { + resolve() + } else { + reject(error) + } + } + const cleanup = (): void => { + signal.removeEventListener('abort', onAbort) + } + signal.addEventListener('abort', onAbort, { once: true }) + setupPromise.then( + () => onSettled(), + (error) => onSettled(error) + ) + }) } export async function registerSshFilesystemWatch(args: { @@ -12,37 +57,79 @@ export async function registerSshFilesystemWatch(args: { registrations: Map rootPath: string callback: (events: FsChangeEvent[]) => void + signal?: AbortSignal }): Promise<() => void> { if (args.disposed()) { throw new Error('SSH filesystem provider disposed') } + if (args.signal?.aborted) { + throw createWatchAbortError() + } + let registration = args.registrations.get(args.rootPath) if (registration) { + if (registration.stopping) { + throw createWatchAbortError() + } registration.callbacks.add(args.callback) - await registration.setupPromise - assertActiveWatch(args, registration) - return createSshFilesystemWatchUnsubscribe(args, registration) + try { + // Why: each caller may leave a shared setup independently; registration + // ownership decides whether the physical relay request should stop. + await awaitSetupWithOptionalAbort(registration.setupPromise, args.signal) + assertActiveWatch(args, registration) + return createSshFilesystemWatchUnsubscribe(args, registration) + } catch (error) { + releaseSshFilesystemWatchCallback(args, registration) + throw error + } } const callbacks = new Set<(events: FsChangeEvent[]) => void>([args.callback]) - const setupPromise = args.mux.request('fs.watch', { rootPath: args.rootPath }).then( - () => undefined, - (error) => { - if (args.registrations.get(args.rootPath) === registration) { - args.registrations.delete(args.rootPath) + const setupAbortController = new AbortController() + registration = { + callbacks, + setupAbortController, + ready: false, + stopping: false, + unwatchSent: false, + setupPromise: Promise.resolve() + } + const createdRegistration = registration + // Why: the shared registration, not its first caller, owns relay setup. + // This keeps a same-root joiner alive when the original caller disconnects. + registration.setupPromise = args.mux + .request('fs.watch', { rootPath: args.rootPath }, { signal: setupAbortController.signal }) + .then( + () => { + createdRegistration.ready = true + if ( + createdRegistration.stopping || + createdRegistration.callbacks.size === 0 || + args.disposed() || + args.registrations.get(args.rootPath) !== createdRegistration + ) { + if (args.registrations.get(args.rootPath) === createdRegistration) { + args.registrations.delete(args.rootPath) + } + sendSshFilesystemUnwatchOnce(args.mux, args.rootPath, createdRegistration) + } + }, + (error) => { + if (args.registrations.get(args.rootPath) === createdRegistration) { + args.registrations.delete(args.rootPath) + } + throw error } - throw error - } - ) - registration = { callbacks, setupPromise } + ) args.registrations.set(args.rootPath, registration) - await setupPromise - if (args.disposed() || args.registrations.get(args.rootPath) !== registration) { - notifySshFilesystemUnwatch(args.mux, args.rootPath) - throw new Error('SSH filesystem provider disposed') + try { + await awaitSetupWithOptionalAbort(registration.setupPromise, args.signal) + assertActiveWatch(args, registration) + return createSshFilesystemWatchUnsubscribe(args, registration) + } catch (error) { + releaseSshFilesystemWatchCallback(args, registration) + throw error } - - return createSshFilesystemWatchUnsubscribe(args, registration) } export function notifySshFilesystemUnwatch(mux: SshChannelMultiplexer, rootPath: string): void { @@ -74,13 +161,52 @@ function createSshFilesystemWatchUnsubscribe( registration: WatchRegistration ): () => void { return () => { - registration.callbacks.delete(args.callback) - if ( - registration.callbacks.size === 0 && - args.registrations.get(args.rootPath) === registration - ) { - args.registrations.delete(args.rootPath) - notifySshFilesystemUnwatch(args.mux, args.rootPath) - } + releaseSshFilesystemWatchCallback(args, registration) } } + +function releaseSshFilesystemWatchCallback( + args: { + mux: SshChannelMultiplexer + registrations: Map + rootPath: string + callback: (events: FsChangeEvent[]) => void + }, + registration: WatchRegistration +): void { + registration.callbacks.delete(args.callback) + if (registration.callbacks.size > 0 || args.registrations.get(args.rootPath) !== registration) { + return + } + registration.stopping = true + if (registration.ready) { + args.registrations.delete(args.rootPath) + sendSshFilesystemUnwatchOnce(args.mux, args.rootPath, registration) + } else { + // Keep the cancelling registration indexed until its request settles so a + // new same-root setup cannot race an old late success and be unwatched. + registration.setupAbortController.abort() + } +} + +export function stopSshFilesystemWatchRegistration( + mux: SshChannelMultiplexer, + rootPath: string, + registration: WatchRegistration +): void { + registration.stopping = true + registration.setupAbortController.abort() + sendSshFilesystemUnwatchOnce(mux, rootPath, registration) +} + +function sendSshFilesystemUnwatchOnce( + mux: SshChannelMultiplexer, + rootPath: string, + registration: WatchRegistration +): void { + if (registration.unwatchSent) { + return + } + registration.unwatchSent = true + notifySshFilesystemUnwatch(mux, rootPath) +} diff --git a/src/main/providers/ssh-filesystem-provider.test.ts b/src/main/providers/ssh-filesystem-provider.test.ts index cc3e890c338..d583a93d7cc 100644 --- a/src/main/providers/ssh-filesystem-provider.test.ts +++ b/src/main/providers/ssh-filesystem-provider.test.ts @@ -538,10 +538,99 @@ describe('SshFilesystemProvider', () => { const callback = vi.fn() const unsub = await provider.watch('/home/user/project', callback) - expect(mux.request).toHaveBeenCalledWith('fs.watch', { rootPath: '/home/user/project' }) + expect(mux.request).toHaveBeenCalledWith( + 'fs.watch', + { rootPath: '/home/user/project' }, + { signal: expect.any(AbortSignal) } + ) expect(typeof unsub).toBe('function') }) + it('uses a registration-owned cancellation signal for the mux fs.watch request', async () => { + mux.request.mockResolvedValue(undefined) + const controller = new AbortController() + const callback = vi.fn() + + await provider.watch('/home/user/project', callback, { signal: controller.signal }) + + expect(mux.request).toHaveBeenCalledWith( + 'fs.watch', + { rootPath: '/home/user/project' }, + { signal: expect.any(AbortSignal) } + ) + const physicalSignal = mux.request.mock.calls[0][2]?.signal + expect(physicalSignal).not.toBe(controller.signal) + expect(physicalSignal?.aborted).toBe(false) + }) + + it('rejects promptly when the watch signal aborts during setup', async () => { + let resolveWatch: () => void = () => {} + mux.request.mockImplementationOnce( + (_method, _params, options?: { signal?: AbortSignal }) => + new Promise((resolve, reject) => { + resolveWatch = resolve + options?.signal?.addEventListener( + 'abort', + () => { + const error = new Error('Request "fs.watch" was cancelled') as Error & { + name: string + } + error.name = 'AbortError' + reject(error) + }, + { once: true } + ) + }) + ) + const controller = new AbortController() + const pendingWatch = provider.watch('/home/user/project', vi.fn(), { + signal: controller.signal + }) + + controller.abort() + + await expect(pendingWatch).rejects.toMatchObject({ name: 'AbortError' }) + expect(mux.request.mock.calls[0][2]?.signal?.aborted).toBe(true) + resolveWatch() + }) + + it('keeps shared setup alive when only its first caller aborts', async () => { + let resolveWatch: () => void = () => {} + let physicalSignal: AbortSignal | undefined + mux.request.mockImplementationOnce( + (_method, _params, options?: { signal?: AbortSignal }) => + new Promise((resolve) => { + resolveWatch = resolve + physicalSignal = options?.signal + }) + ) + const firstController = new AbortController() + const first = vi.fn() + const second = vi.fn() + + const firstWatch = provider.watch('/home/user/project', first, { + signal: firstController.signal + }) + const secondWatch = provider.watch('/home/user/project', second) + firstController.abort() + + await expect(firstWatch).rejects.toMatchObject({ name: 'AbortError' }) + expect(physicalSignal?.aborted).toBe(false) + + resolveWatch() + const unsubscribeSecond = await secondWatch + const notifHandler = mux.onNotification.mock.calls[0][0] + const events = [{ kind: 'update', absolutePath: '/home/user/project/file.ts' }] + notifHandler('fs.changed', { events }) + + expect(first).not.toHaveBeenCalled() + expect(second).toHaveBeenCalledWith(events) + unsubscribeSecond() + expect(mux.notify).toHaveBeenCalledWith('fs.unwatch', { + rootPath: '/home/user/project' + }) + }) + it('forwards fs.changed notifications to watch callback', async () => { const callback = vi.fn() await provider.watch('/home/user/project', callback) @@ -560,7 +649,11 @@ describe('SshFilesystemProvider', () => { const unsubSecond = await provider.watch('/home/user/project', second) expect(mux.request).toHaveBeenCalledTimes(1) - expect(mux.request).toHaveBeenCalledWith('fs.watch', { rootPath: '/home/user/project' }) + expect(mux.request).toHaveBeenCalledWith( + 'fs.watch', + { rootPath: '/home/user/project' }, + { signal: expect.any(AbortSignal) } + ) const notifHandler = mux.onNotification.mock.calls[0][0] const events = [{ kind: 'update', absolutePath: '/home/user/project/file.ts' }] diff --git a/src/main/providers/ssh-filesystem-provider.ts b/src/main/providers/ssh-filesystem-provider.ts index cb95683f9fd..ed4520e8f5a 100644 --- a/src/main/providers/ssh-filesystem-provider.ts +++ b/src/main/providers/ssh-filesystem-provider.ts @@ -8,8 +8,8 @@ import { type SshRawTransferOptions } from './ssh-filesystem-file-upload' import { - notifySshFilesystemUnwatch, registerSshFilesystemWatch, + stopSshFilesystemWatchRegistration, type WatchRegistration } from './ssh-filesystem-provider-watch' import type { @@ -66,8 +66,8 @@ export class SshFilesystemProvider implements IFilesystemProvider { this.unsubscribeNotifications() this.unsubscribeNotifications = null } - for (const rootPath of this.watchListeners.keys()) { - notifySshFilesystemUnwatch(this.mux, rootPath) + for (const [rootPath, registration] of this.watchListeners) { + stopSshFilesystemWatchRegistration(this.mux, rootPath, registration) } this.watchListeners.clear() } @@ -316,13 +316,18 @@ export class SshFilesystemProvider implements IFilesystemProvider { })) as string[] } - async watch(rootPath: string, callback: (events: FsChangeEvent[]) => void): Promise<() => void> { + async watch( + rootPath: string, + callback: (events: FsChangeEvent[]) => void, + options?: { signal?: AbortSignal } + ): Promise<() => void> { return registerSshFilesystemWatch({ mux: this.mux, disposed: () => this.disposed, registrations: this.watchListeners, rootPath, - callback + callback, + signal: options?.signal }) } } diff --git a/src/main/providers/types.ts b/src/main/providers/types.ts index d5cf5c60985..66642e38ad1 100644 --- a/src/main/providers/types.ts +++ b/src/main/providers/types.ts @@ -279,7 +279,11 @@ export type IFilesystemProvider = { rootPath: string, options?: { signal?: AbortSignal } ): Promise - watch(rootPath: string, callback: (events: FsChangeEvent[]) => void): Promise<() => void> + watch( + rootPath: string, + callback: (events: FsChangeEvent[]) => void, + options?: { signal?: AbortSignal } + ): Promise<() => void> } export type FileUploadSession = { diff --git a/src/main/runtime/file-watcher-host.test.ts b/src/main/runtime/file-watcher-host.test.ts index 6abea97bac7..e64a7e9d145 100644 --- a/src/main/runtime/file-watcher-host.test.ts +++ b/src/main/runtime/file-watcher-host.test.ts @@ -1,236 +1,301 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { FsChangeEvent } from '../../shared/types' +import type { + WatcherProcessCallback, + WatcherProcessHooks, + WatcherProcessSubscription +} from '../ipc/parcel-watcher-process' +import { WatcherProcessFailure } from '../ipc/parcel-watcher-process-failure' +import { + WATCHER_IGNORE_DIRS, + buildParcelWatcherIgnoreOptions +} from '../ipc/filesystem-watcher-ignore' -type MockWorker = { - terminated: boolean - postedMessages: unknown[] - workerData: unknown - on(event: string, listener: (arg?: unknown) => void): MockWorker - once(event: string, listener: (arg?: unknown) => void): MockWorker - off(event: string, listener: (arg?: unknown) => void): MockWorker - postMessage(message: unknown): void - terminate(): Promise - emit(event: string, arg?: unknown): void - listenerCount(event: string): number +const { + forgetRuntimeWatcherProcessRootMock, + resetRuntimeWatcherProcessForTestMock, + subscribeViaRuntimeWatcherProcessMock +} = vi.hoisted(() => ({ + forgetRuntimeWatcherProcessRootMock: vi.fn(), + resetRuntimeWatcherProcessForTestMock: vi.fn(), + subscribeViaRuntimeWatcherProcessMock: vi.fn() +})) + +vi.mock('../ipc/parcel-watcher-process', () => ({ + forgetRuntimeWatcherProcessRoot: forgetRuntimeWatcherProcessRootMock, + resetRuntimeWatcherProcessForTest: resetRuntimeWatcherProcessForTestMock, + subscribeViaRuntimeWatcherProcess: subscribeViaRuntimeWatcherProcessMock +})) + +import { + resetRuntimeRootWatchersForTest, + watchFileExplorerInWatcherProcess +} from './file-watcher-host' + +type InstalledWatch = { + callback: WatcherProcessCallback + hooks: WatcherProcessHooks + unsubscribe: ReturnType Promise>> } -const workerState = vi.hoisted(() => { - const instances: MockWorker[] = [] - class MockWorkerImpl { - terminated = false - postedMessages: unknown[] = [] - workerData: unknown - private listeners = new Map void; once: boolean }[]>() - - constructor(_workerPath: string, options: { workerData?: unknown }) { - this.workerData = options.workerData - instances.push(this as unknown as MockWorker) +function installSuccessfulWatch(): InstalledWatch { + let callback: WatcherProcessCallback | undefined + let hooks: WatcherProcessHooks | undefined + const unsubscribe = vi.fn(async () => undefined) + subscribeViaRuntimeWatcherProcessMock.mockImplementationOnce( + async ( + _rootPath: string, + nextCallback: WatcherProcessCallback, + _options: unknown, + nextHooks: WatcherProcessHooks + ): Promise => { + callback = nextCallback + hooks = nextHooks + return { unsubscribe } } - - on(event: string, listener: (arg?: unknown) => void): this { - const list = this.listeners.get(event) ?? [] - list.push({ listener, once: false }) - this.listeners.set(event, list) - return this - } - - once(event: string, listener: (arg?: unknown) => void): this { - const list = this.listeners.get(event) ?? [] - list.push({ listener, once: true }) - this.listeners.set(event, list) - return this - } - - off(event: string, listener: (arg?: unknown) => void): this { - const list = this.listeners.get(event) ?? [] - this.listeners.set( - event, - list.filter((entry) => entry.listener !== listener) - ) - return this - } - - postMessage(message: unknown): void { - this.postedMessages.push(message) - } - - async terminate(): Promise { - this.terminated = true - return 0 - } - - emit(event: string, arg?: unknown): void { - const entries = this.listeners.get(event)?.slice() ?? [] - for (const entry of entries) { - if (entry.once) { - this.off(event, entry.listener) - } - entry.listener(arg) + ) + return { + get callback() { + if (!callback) { + throw new Error('watch callback not installed') } - } - - listenerCount(event: string): number { - return this.listeners.get(event)?.length ?? 0 - } + return callback + }, + get hooks() { + if (!hooks) { + throw new Error('watch hooks not installed') + } + return hooks + }, + unsubscribe } - return { instances, MockWorkerImpl } -}) - -vi.mock('electron', () => ({ - app: { isPackaged: false } -})) - -vi.mock('worker_threads', () => ({ - Worker: workerState.MockWorkerImpl -})) - -import { watchFileExplorerInWorker } from './file-watcher-host' - -function lastWorker(): MockWorker { - const worker = workerState.instances.at(-1) - if (!worker) { - throw new Error('no worker spawned') - } - return worker } -describe('watchFileExplorerInWorker', () => { +describe('watchFileExplorerInWatcherProcess', () => { beforeEach(() => { - workerState.instances.length = 0 + forgetRuntimeWatcherProcessRootMock.mockReset() + resetRuntimeWatcherProcessForTestMock.mockReset() + subscribeViaRuntimeWatcherProcessMock.mockReset() }) afterEach(() => { + resetRuntimeRootWatchersForTest() vi.restoreAllMocks() }) - it('resolves to an unsubscribe fn once the worker reports ready', async () => { - const promise = watchFileExplorerInWorker('/repo', vi.fn()) - const worker = lastWorker() - expect(worker.workerData).toMatchObject({ rootPath: '/repo' }) + it('installs a bounded watch in the shared runtime watcher process', async () => { + const watch = installSuccessfulWatch() - worker.emit('message', { type: 'ready' }) - const dispose = await promise - expect(typeof dispose).toBe('function') - }) + const dispose = await watchFileExplorerInWatcherProcess('/repo', vi.fn()) - it('forwards worker events to the callback only after ready', async () => { - const onEvents = vi.fn<(events: FsChangeEvent[]) => void>() - const promise = watchFileExplorerInWorker('/repo', onEvents) - const worker = lastWorker() - worker.emit('message', { type: 'ready' }) - await promise - - const events: FsChangeEvent[] = [ - { kind: 'update', absolutePath: '/repo/a.txt', isDirectory: false } - ] - worker.emit('message', { type: 'events', events }) - expect(onEvents).toHaveBeenCalledWith(events) - }) - - it('rejects if the worker errors before the crawl goes live', async () => { - const promise = watchFileExplorerInWorker('/repo', vi.fn()) - const worker = lastWorker() - worker.emit('message', { type: 'error', message: 'addon missing' }) - - await expect(promise).rejects.toThrow('addon missing') - expect(worker.terminated).toBe(true) - }) - - it('rejects if the worker exits before ready', async () => { - const promise = watchFileExplorerInWorker('/repo', vi.fn()) - const worker = lastWorker() - worker.emit('exit', 1) - - await expect(promise).rejects.toThrow(/exited before ready/) - }) - - it('emits an overflow if a live worker crashes', async () => { - const onEvents = vi.fn<(events: FsChangeEvent[]) => void>() - const promise = watchFileExplorerInWorker('/repo', onEvents) - const worker = lastWorker() - worker.emit('message', { type: 'ready' }) - await promise - - worker.emit('error', new Error('boom')) - expect(onEvents).toHaveBeenCalledWith([{ kind: 'overflow', absolutePath: '/repo' }]) - }) - - it('unsubscribes and waits for a clean worker exit without force-terminating', async () => { - const promise = watchFileExplorerInWorker('/repo', vi.fn()) - const worker = lastWorker() - worker.emit('message', { type: 'ready' }) - const dispose = await promise - - const disposed = dispose() - expect(worker.postedMessages).toContainEqual({ type: 'unsubscribe' }) - // The worker unsubscribes its native watcher, closes its port and exits on - // its own — no force terminate, which is what corrupts the native watcher. - worker.emit('exit', 0) - await disposed - expect(worker.terminated).toBe(false) - expect(worker.listenerCount('exit')).toBe(1) - - // Idempotent: a second dispose does nothing further. - await dispose() - expect( - worker.postedMessages.filter((m) => (m as { type?: string }).type === 'unsubscribe') - ).toHaveLength(1) - }) - - it('shares pending dispose work across racing callers', async () => { - const promise = watchFileExplorerInWorker('/repo', vi.fn()) - const worker = lastWorker() - worker.emit('message', { type: 'ready' }) - const dispose = await promise + expect(subscribeViaRuntimeWatcherProcessMock).toHaveBeenCalledWith( + '/repo', + expect.any(Function), + buildParcelWatcherIgnoreOptions(WATCHER_IGNORE_DIRS), + expect.objectContaining({ + delivery: { includeDirectoryMetadata: true, maxEventsPerBatch: 200 }, + onInterruption: expect.any(Function), + onOverflow: expect.any(Function), + onTerminalError: expect.any(Function), + signal: expect.any(AbortSignal) + }) + ) const firstDispose = dispose() const secondDispose = dispose() expect(secondDispose).toBe(firstDispose) - expect( - worker.postedMessages.filter((m) => (m as { type?: string }).type === 'unsubscribe') - ).toHaveLength(1) - - worker.emit('exit', 0) - await Promise.all([firstDispose, secondDispose]) - expect(worker.terminated).toBe(false) + await firstDispose + expect(watch.unsubscribe).toHaveBeenCalledTimes(1) }) - it('force-terminates the worker only if it fails to exit within the timeout', async () => { - vi.useFakeTimers() - try { - const promise = watchFileExplorerInWorker('/repo', vi.fn()) - const worker = lastWorker() - worker.emit('message', { type: 'ready' }) - const dispose = await promise - - const disposed = dispose() - expect(worker.postedMessages).toContainEqual({ type: 'unsubscribe' }) - expect(worker.listenerCount('exit')).toBe(2) - // Worker is wedged and never emits exit: the backstop must terminate it. - await vi.advanceTimersByTimeAsync(10_000) - await disposed - expect(worker.terminated).toBe(true) - expect(worker.listenerCount('exit')).toBe(1) - } finally { - vi.useRealTimers() - } - }) - - it('stops forwarding events after dispose', async () => { + it('maps child-enriched watcher events without host-process stat work', async () => { + const watch = installSuccessfulWatch() const onEvents = vi.fn<(events: FsChangeEvent[]) => void>() - const promise = watchFileExplorerInWorker('/repo', onEvents) - const worker = lastWorker() - worker.emit('message', { type: 'ready' }) - const dispose = await promise - const disposed = dispose() - worker.emit('exit', 0) - await disposed - onEvents.mockClear() + await watchFileExplorerInWatcherProcess('/repo', onEvents) + + watch.callback(null, [ + { type: 'update', path: '/repo/file.txt', isDirectory: false }, + { type: 'create', path: '/repo/dir', isDirectory: true }, + { type: 'delete', path: '/repo/gone.txt' } + ]) + + expect(onEvents).toHaveBeenCalledWith([ + { kind: 'update', absolutePath: '/repo/file.txt', isDirectory: false }, + { kind: 'create', absolutePath: '/repo/dir', isDirectory: true }, + { kind: 'delete', absolutePath: '/repo/gone.txt', isDirectory: undefined } + ]) + }) + + it('turns child overflow and recoverable watcher errors into a conservative refresh', async () => { + const watch = installSuccessfulWatch() + const onEvents = vi.fn<(events: FsChangeEvent[]) => void>() + vi.spyOn(console, 'error').mockImplementation(() => undefined) + await watchFileExplorerInWatcherProcess('/repo', onEvents) + + watch.hooks.onOverflow?.() + watch.callback(new Error('Events were dropped by the FSEvents client.'), []) + + expect(onEvents).toHaveBeenNthCalledWith(1, [{ kind: 'overflow', absolutePath: '/repo' }]) + expect(onEvents).toHaveBeenNthCalledWith(2, [{ kind: 'overflow', absolutePath: '/repo' }]) + }) + + it('refreshes only after the child has resubscribed', async () => { + const watch = installSuccessfulWatch() + const onEvents = vi.fn<(events: FsChangeEvent[]) => void>() + await watchFileExplorerInWatcherProcess('/repo', onEvents) + + watch.hooks.onInterruption?.() + + expect(onEvents).toHaveBeenCalledWith([{ kind: 'overflow', absolutePath: '/repo' }]) + }) + + it('keeps same-root subscribers alive while a failed shard is replaced', async () => { + const watch = installSuccessfulWatch() + const replacement = installSuccessfulWatch() + const firstEvents = vi.fn<(events: FsChangeEvent[]) => void>() + const secondEvents = vi.fn<(events: FsChangeEvent[]) => void>() + const firstError = vi.fn() + const secondError = vi.fn() + await watchFileExplorerInWatcherProcess('/repo', firstEvents, firstError) + await watchFileExplorerInWatcherProcess('/repo', secondEvents, secondError) + + watch.hooks.onTerminalError?.( + new WatcherProcessFailure('crashed repeatedly', 'supervisor', 'supervisor_crash_fuse') + ) + await vi.waitFor(() => expect(subscribeViaRuntimeWatcherProcessMock).toHaveBeenCalledTimes(2)) + watch.callback(null, [{ type: 'update', path: '/repo/late.txt' }]) + replacement.callback(null, [{ type: 'update', path: '/repo/recovered.txt' }]) + + expect(firstError).not.toHaveBeenCalled() + expect(secondError).not.toHaveBeenCalled() + expect(firstEvents).toHaveBeenCalledWith([{ kind: 'overflow', absolutePath: '/repo' }]) + expect(firstEvents).toHaveBeenCalledWith([ + { kind: 'update', absolutePath: '/repo/recovered.txt', isDirectory: undefined } + ]) + expect(secondEvents).toHaveBeenCalledTimes(2) + }) + + it('ends subscribers only after isolated recovery also fails', async () => { + const watch = installSuccessfulWatch() + subscribeViaRuntimeWatcherProcessMock.mockRejectedValueOnce(new Error('isolated failure')) + const onEvents = vi.fn<(events: FsChangeEvent[]) => void>() + const onTerminalError = vi.fn() + await watchFileExplorerInWatcherProcess('/repo', onEvents, onTerminalError) + + watch.hooks.onTerminalError?.( + new WatcherProcessFailure('crashed repeatedly', 'supervisor', 'supervisor_crash_fuse') + ) + + await vi.waitFor(() => + expect(onTerminalError).toHaveBeenCalledWith( + expect.objectContaining({ message: 'isolated failure' }) + ) + ) + expect(onEvents).toHaveBeenCalledWith([{ kind: 'overflow', absolutePath: '/repo' }]) + }) + + it('cancels a pending physical watch when its only client aborts', async () => { + let physicalSignal: AbortSignal | undefined + subscribeViaRuntimeWatcherProcessMock.mockImplementation( + (_rootPath, _callback, _options, hooks: WatcherProcessHooks) => + new Promise((_resolve, reject) => { + physicalSignal = hooks.signal + hooks.signal?.addEventListener( + 'abort', + () => reject(new Error('physical watch aborted')), + { once: true } + ) + }) + ) + const controller = new AbortController() + const pending = watchFileExplorerInWatcherProcess('/repo', vi.fn(), vi.fn(), controller.signal) + + controller.abort() + + await expect(pending).rejects.toThrow('aborted') + expect(physicalSignal?.aborted).toBe(true) + }) + + it('keeps pending setup alive when another same-root client still owns it', async () => { + let physicalSignal: AbortSignal | undefined + let resolvePhysical: ((subscription: WatcherProcessSubscription) => void) | undefined + const unsubscribe = vi.fn(async () => undefined) + subscribeViaRuntimeWatcherProcessMock.mockImplementation( + (_rootPath, _callback, _options, hooks: WatcherProcessHooks) => { + physicalSignal = hooks.signal + return new Promise((resolve) => { + resolvePhysical = resolve + }) + } + ) + const controller = new AbortController() + const first = watchFileExplorerInWatcherProcess('/repo', vi.fn(), vi.fn(), controller.signal) + const second = watchFileExplorerInWatcherProcess('/repo', vi.fn()) + + controller.abort() + await expect(first).rejects.toThrow('aborted') + expect(physicalSignal?.aborted).toBe(false) + + resolvePhysical?.({ unsubscribe }) + await expect(second).resolves.toEqual(expect.any(Function)) + expect(subscribeViaRuntimeWatcherProcessMock).toHaveBeenCalledTimes(1) + }) + + it('shares one native subscription per root across same-root callers', async () => { + const firstRoot = installSuccessfulWatch() + const secondRoot = installSuccessfulWatch() + + await watchFileExplorerInWatcherProcess('/repo-a', vi.fn()) + await watchFileExplorerInWatcherProcess('/repo-a', vi.fn()) + await watchFileExplorerInWatcherProcess('/repo-b', vi.fn()) + + expect(subscribeViaRuntimeWatcherProcessMock).toHaveBeenCalledTimes(2) + expect(subscribeViaRuntimeWatcherProcessMock).toHaveBeenNthCalledWith( + 1, + '/repo-a', + expect.any(Function), + expect.any(Object), + expect.any(Object) + ) + expect(subscribeViaRuntimeWatcherProcessMock).toHaveBeenNthCalledWith( + 2, + '/repo-b', + expect.any(Function), + expect.any(Object), + expect.any(Object) + ) + expect(firstRoot.unsubscribe).not.toHaveBeenCalled() + expect(secondRoot.unsubscribe).not.toHaveBeenCalled() + }) + + it('stops forwarding events after the last subscriber disposes', async () => { + const watch = installSuccessfulWatch() + const onEvents = vi.fn<(events: FsChangeEvent[]) => void>() + const dispose = await watchFileExplorerInWatcherProcess('/repo', onEvents) + + await dispose() + watch.callback(null, [{ type: 'update', path: '/repo/a.txt' }]) + watch.hooks.onInterruption?.() - worker.emit('message', { - type: 'events', - events: [{ kind: 'update', absolutePath: '/repo/a.txt' }] - }) expect(onEvents).not.toHaveBeenCalled() }) + + it('propagates an initial watcher process subscription failure', async () => { + subscribeViaRuntimeWatcherProcessMock.mockRejectedValue(new Error('watcher unavailable')) + + await expect(watchFileExplorerInWatcherProcess('/repo', vi.fn())).rejects.toThrow( + 'watcher unavailable' + ) + }) + + it('does not retry a root-specific initial subscription failure', async () => { + subscribeViaRuntimeWatcherProcessMock.mockRejectedValue( + new WatcherProcessFailure('root unavailable', 'subscription', 'subscribe_failed') + ) + + await expect(watchFileExplorerInWatcherProcess('/missing', vi.fn())).rejects.toThrow( + 'root unavailable' + ) + expect(subscribeViaRuntimeWatcherProcessMock).toHaveBeenCalledTimes(1) + }) }) diff --git a/src/main/runtime/file-watcher-host.ts b/src/main/runtime/file-watcher-host.ts index f368ea270e2..68e5711150d 100644 --- a/src/main/runtime/file-watcher-host.ts +++ b/src/main/runtime/file-watcher-host.ts @@ -1,160 +1,286 @@ -// Why: spawns the file-watcher worker thread and adapts it to the synchronous -// `watchFileExplorer` contract (a promise that resolves to an unsubscribe fn -// once the recursive crawl is live). Running @parcel/watcher in the worker -// keeps its blocking initial crawl off the main process's libuv pool so a huge -// non-git tree can't wedge the `serve` runtime (issue #5308). -import { Worker } from 'node:worker_threads' -import { join } from 'node:path' -import { app } from 'electron' import type { FsChangeEvent } from '../../shared/types' -import type { FileWatcherHostMessage, FileWatcherWorkerMessage } from './file-watcher-worker' -// Why: shares the explorer watcher's canonical ignore list, pre-shaped so the -// macOS daemon-side exclusion-path cap (8) is respected — over the cap ALL -// exclusions silently fail and fseventsd delivers full node_modules churn. +import { + forgetRuntimeWatcherProcessRoot, + resetRuntimeWatcherProcessForTest, + subscribeViaRuntimeWatcherProcess, + type WatcherProcessEvent, + type WatcherProcessSubscription +} from '../ipc/parcel-watcher-process' +import { isWatcherProcessFailure } from '../ipc/parcel-watcher-process-failure' import { WATCHER_IGNORE_DIRS, buildParcelWatcherIgnoreOptions } from '../ipc/filesystem-watcher-ignore' const RUNTIME_FILE_WATCH_IGNORE_OPTIONS = buildParcelWatcherIgnoreOptions(WATCHER_IGNORE_DIRS) +const RUNTIME_FILE_WATCH_EVENT_LIMIT = 200 +const RUNTIME_FILE_WATCH_CRAWL_TIMEOUT_MS = 60_000 -// Why: clean teardown is async (the worker awaits subscription.unsubscribe() -// before closing its port and exiting). Wait this long for the worker to exit on -// its own before force-terminating, so the native watcher thread isn't freed -// mid-flight. -const WORKER_TEARDOWN_TIMEOUT_MS = 5000 -type WorkerExitWaitResult = 'exit' | 'timeout' - -function getFileWatcherWorkerPath(): string { - if (app.isPackaged) { - return join(process.resourcesPath, 'app.asar', 'out', 'main', 'file-watcher-worker.js') - } - return join(__dirname, 'file-watcher-worker.js') +type RuntimeFileWatchSubscriber = { + onEvents: (events: FsChangeEvent[]) => void + onTerminalError: (error: Error) => void } -function waitForWorkerExit(worker: Worker, timeoutMs: number): Promise { - return new Promise((resolve) => { +type RuntimeRootWatch = { + rootPath: string + subscribers: Set + start: Promise + subscription: WatcherProcessSubscription | null + abortController: AbortController + generation: number + closed: boolean +} + +// Why: paired clients watching the same worktree share one native subscription. +// Different roots are assigned across the bounded crash-isolated child pool. +const runtimeRootWatches = new Map() + +function overflowEvent(rootPath: string): FsChangeEvent[] { + return [{ kind: 'overflow', absolutePath: rootPath }] +} + +function mapWatcherEvents(events: readonly WatcherProcessEvent[]): FsChangeEvent[] { + return events.map((event) => ({ + kind: event.type, + absolutePath: event.path, + isDirectory: event.isDirectory + })) +} + +function emitToSubscribers(root: RuntimeRootWatch, events: FsChangeEvent[]): void { + if (root.closed) { + return + } + for (const subscriber of root.subscribers) { + subscriber.onEvents(events) + } +} + +function terminateRootWatch(root: RuntimeRootWatch, error: Error): void { + if (root.closed) { + return + } + root.closed = true + root.generation++ + root.abortController.abort() + if (runtimeRootWatches.get(root.rootPath) === root) { + runtimeRootWatches.delete(root.rootPath) + } + forgetRuntimeWatcherProcessRoot(root.rootPath) + const subscribers = Array.from(root.subscribers) + root.subscribers.clear() + for (const subscriber of subscribers) { + subscriber.onTerminalError(error) + } +} + +function closeInitialRootWatch(root: RuntimeRootWatch): void { + root.closed = true + root.generation++ + root.abortController.abort() + if (runtimeRootWatches.get(root.rootPath) === root) { + runtimeRootWatches.delete(root.rootPath) + } + forgetRuntimeWatcherProcessRoot(root.rootPath) + root.subscribers.clear() +} + +function shouldRetryInitialWatch(error: unknown): boolean { + return ( + isWatcherProcessFailure(error) && + error.code !== 'entry_missing' && + error.code !== 'subscribe_aborted' && + error.code !== 'supervisor_disposed' && + (error.scope === 'supervisor' || error.code === 'subscribe_timeout') + ) +} + +function subscribeRuntimeRootWatch(root: RuntimeRootWatch): Promise { + const generation = ++root.generation + const emitOverflow = (): void => { + if (root.generation === generation) { + emitToSubscribers(root, overflowEvent(root.rootPath)) + } + } + return subscribeViaRuntimeWatcherProcess( + root.rootPath, + (err, events) => { + if (root.generation !== generation || root.closed) { + return + } + if (err) { + console.error('[runtime-files.watch] watcher error', { + rootPath: root.rootPath, + error: err.message + }) + emitOverflow() + return + } + emitToSubscribers(root, mapWatcherEvents(events)) + }, + RUNTIME_FILE_WATCH_IGNORE_OPTIONS, + { + delivery: { + includeDirectoryMetadata: true, + maxEventsPerBatch: RUNTIME_FILE_WATCH_EVENT_LIMIT + }, + onInterruption: emitOverflow, + onOverflow: emitOverflow, + onTerminalError: (error) => recoverRuntimeRootWatch(root, generation, error), + // Why: the transport bounds initial setup at 15 seconds, but a later + // crash-resubscribe has no request deadline and must not freeze its shard forever. + subscribeTimeoutMs: RUNTIME_FILE_WATCH_CRAWL_TIMEOUT_MS, + signal: root.abortController.signal + } + ).then(async (subscription) => { + if (root.closed || root.generation !== generation) { + await subscription.unsubscribe() + return + } + root.subscription = subscription + }) +} + +async function startInitialRuntimeRootWatch(root: RuntimeRootWatch): Promise { + try { + await subscribeRuntimeRootWatch(root) + } catch (firstError) { + if (root.closed || !shouldRetryInitialWatch(firstError)) { + closeInitialRootWatch(root) + throw firstError + } + try { + await subscribeRuntimeRootWatch(root) + emitToSubscribers(root, overflowEvent(root.rootPath)) + } catch (isolatedError) { + closeInitialRootWatch(root) + throw isolatedError + } + } +} + +function recoverRuntimeRootWatch( + root: RuntimeRootWatch, + failedGeneration: number, + terminalError: Error +): void { + if (root.closed || root.generation !== failedGeneration) { + return + } + root.subscription = null + emitToSubscribers(root, overflowEvent(root.rootPath)) + root.start = subscribeRuntimeRootWatch(root).catch((recoveryError: unknown) => { + if (!root.closed) { + terminateRootWatch(root, recoveryError instanceof Error ? recoveryError : terminalError) + } + }) +} + +function createRuntimeRootWatch(rootPath: string): RuntimeRootWatch { + const root: RuntimeRootWatch = { + rootPath, + subscribers: new Set(), + start: Promise.resolve(), + subscription: null, + abortController: new AbortController(), + generation: 0, + closed: false + } + runtimeRootWatches.set(rootPath, root) + root.start = startInitialRuntimeRootWatch(root) + return root +} + +async function releaseRuntimeRootWatch( + root: RuntimeRootWatch, + subscriber: RuntimeFileWatchSubscriber +): Promise { + root.subscribers.delete(subscriber) + if (root.closed || root.subscribers.size > 0) { + return + } + root.closed = true + root.generation++ + root.abortController.abort() + if (runtimeRootWatches.get(root.rootPath) === root) { + runtimeRootWatches.delete(root.rootPath) + } + await root.subscription?.unsubscribe() +} + +/** + * Start a recursive runtime-file watch in the crash-isolated process pool. + * The child owns crawl, event collapse, and stat metadata so neither native + * faults nor event storms can consume the serve process's libuv pool. + */ +export async function watchFileExplorerInWatcherProcess( + rootPath: string, + callback: (events: FsChangeEvent[]) => void, + onTerminalError: (error: Error) => void = () => undefined, + signal?: AbortSignal +): Promise<() => Promise> { + const root = runtimeRootWatches.get(rootPath) ?? createRuntimeRootWatch(rootPath) + const subscriber: RuntimeFileWatchSubscriber = { onEvents: callback, onTerminalError } + root.subscribers.add(subscriber) + try { + await waitForRuntimeRootStart(root, subscriber, signal) + } catch (error) { + root.subscribers.delete(subscriber) + throw error + } + if (root.closed) { + throw new Error('file watcher closed during setup') + } + + let releasePromise: Promise | undefined + return (): Promise => { + releasePromise ??= releaseRuntimeRootWatch(root, subscriber) + return releasePromise + } +} + +function waitForRuntimeRootStart( + root: RuntimeRootWatch, + subscriber: RuntimeFileWatchSubscriber, + signal: AbortSignal | undefined +): Promise { + if (!signal) { + return root.start + } + if (signal.aborted) { + void releaseRuntimeRootWatch(root, subscriber) + return Promise.reject(new Error('file watcher subscription aborted')) + } + return new Promise((resolve, reject) => { let settled = false - let timer: ReturnType | undefined - let onExit: (() => void) | undefined - const finish = (result: WorkerExitWaitResult): void => { + const finish = (callback: () => void): void => { if (settled) { return } settled = true - if (timer) { - clearTimeout(timer) - } - if (onExit) { - worker.off('exit', onExit) - } - resolve(result) + signal.removeEventListener('abort', handleAbort) + callback() } - - onExit = () => finish('exit') - worker.once('exit', onExit) - timer = setTimeout(() => finish('timeout'), timeoutMs) + const handleAbort = (): void => { + void releaseRuntimeRootWatch(root, subscriber) + finish(() => reject(new Error('file watcher subscription aborted'))) + } + signal.addEventListener('abort', handleAbort, { once: true }) + root.start.then( + () => finish(resolve), + (error: unknown) => finish(() => reject(error)) + ) }) } -/** Start a recursive file watch in a worker thread. Resolves to an unsubscribe - * function once the worker reports the crawl is live; rejects if the worker - * fails to start the watch. */ -export function watchFileExplorerInWorker( - rootPath: string, - callback: (events: FsChangeEvent[]) => void -): Promise<() => Promise> { - return new Promise((resolve, reject) => { - const worker = new Worker(getFileWatcherWorkerPath(), { - workerData: { rootPath, ignoreOptions: RUNTIME_FILE_WATCH_IGNORE_OPTIONS } - }) - - let ready = false - let disposed = false - let exited = false - let disposePromise: Promise | undefined - - const runDispose = async (): Promise => { - if (disposed) { - return - } - disposed = true - if (exited) { - return - } - // Ask the worker to unsubscribe its native watcher and exit on its own. - // Why: worker.terminate() force-frees the worker's V8 env while - // @parcel/watcher's native watch thread / inflight async work is still - // live, which faults inside napi (Watcher::findCallback, - // PromiseRunner::onWorkComplete). Only terminate as a backstop if the - // worker wedges and never exits. - try { - worker.postMessage({ type: 'unsubscribe' } satisfies FileWatcherHostMessage) - } catch { - // Worker already gone — the exit wait and timeout backstop cover it. - } - const exitResult = await waitForWorkerExit(worker, WORKER_TEARDOWN_TIMEOUT_MS) - if (exitResult === 'timeout' && !exited) { - await worker.terminate().then( - () => undefined, - () => undefined - ) - } - } - - // Why: racing dispose callers must share the same worker-exit drain instead - // of letting later calls resolve while teardown is still in flight. - const dispose = (): Promise => { - disposePromise ??= runDispose() - return disposePromise - } - - worker.on('message', (message: FileWatcherWorkerMessage) => { - if (message.type === 'ready') { - ready = true - resolve(dispose) - return - } - if (message.type === 'events') { - if (!disposed) { - callback(message.events) - } - return - } - if (message.type === 'error') { - if (!ready) { - // The crawl never went live — fail the watch so the caller knows. - disposed = true - void worker.terminate() - reject(new Error(message.message)) - return - } - // Already live: a mid-stream watcher error. Tell the renderer to - // refresh; the worker also emits an overflow event alongside this. - console.error('[runtime-files.watch] worker error', { rootPath, error: message.message }) - } - }) - - worker.on('error', (err) => { - if (!ready) { - disposed = true - reject(err) - return - } - // A live worker crashed: surface an overflow so the renderer re-reads, - // rather than silently going stale. - console.error('[runtime-files.watch] worker crashed', { rootPath, err }) - if (!disposed) { - callback([{ kind: 'overflow', absolutePath: rootPath }]) - } - }) - - worker.on('exit', (code) => { - exited = true - if (!ready && !disposed) { - disposed = true - reject(new Error(`file watcher worker exited before ready (code ${code})`)) - } - }) - }) +export function resetRuntimeRootWatchersForTest(): void { + for (const root of runtimeRootWatches.values()) { + root.closed = true + root.generation++ + root.abortController.abort() + root.subscribers.clear() + void root.subscription?.unsubscribe() + } + runtimeRootWatches.clear() + resetRuntimeWatcherProcessForTest() } diff --git a/src/main/runtime/file-watcher-worker.ts b/src/main/runtime/file-watcher-worker.ts deleted file mode 100644 index 9fd8e17cb77..00000000000 --- a/src/main/runtime/file-watcher-worker.ts +++ /dev/null @@ -1,141 +0,0 @@ -// Why: on Linux/Windows @parcel/watcher uses a brute-force backend that -// recursively walks the whole tree on a libuv threadpool thread before -// subscribe() resolves. On a huge tree backed by slow storage (a home dir on -// NFS opened as a worktree) that crawl can run for minutes. Running it here, in -// a dedicated worker thread, keeps it off the main/`serve` process's libuv pool -// so it can never starve static-asset serving, RPC crypto, or other clients -// (issue #5308). The worker owns the subscribe, the per-event stat fanout, and -// the event batching; the main thread only relays results. -import { stat } from 'node:fs/promises' -import { parentPort, workerData } from 'node:worker_threads' -import type * as ParcelWatcher from '@parcel/watcher' -import type { FsChangeEvent } from '../../shared/types' -import type { ParcelWatcherIgnoreOptions } from '../ipc/filesystem-watcher-ignore' - -const RUNTIME_FILE_WATCH_EVENT_STAT_LIMIT = 200 -const RUNTIME_FILE_WATCH_STAT_CONCURRENCY = 8 - -type FileWatcherWorkerData = { - rootPath: string - ignoreOptions: ParcelWatcherIgnoreOptions -} - -// Messages the worker sends back to the host. -export type FileWatcherWorkerMessage = - | { type: 'ready' } - | { type: 'events'; events: FsChangeEvent[] } - | { type: 'error'; message: string } - -// Messages the host sends to the worker. -export type FileWatcherHostMessage = { type: 'unsubscribe' } - -const data = workerData as FileWatcherWorkerData - -if (!parentPort) { - throw new Error('File watcher worker must run with a parent port.') -} - -const port = parentPort - -/** Report a watcher failure to the host and ask the renderer to refresh from - * scratch (the overflow event), so a mid-stream error never leaves the - * explorer silently stale. */ -function reportWatchError(err: unknown): void { - port.postMessage({ - type: 'error', - message: err instanceof Error ? err.message : String(err) - } satisfies FileWatcherWorkerMessage) - port.postMessage({ - type: 'events', - events: [{ kind: 'overflow', absolutePath: data.rootPath }] - } satisfies FileWatcherWorkerMessage) -} - -/** Run an async mapper over items with a bounded number in flight at once, so a - * large batch can't occupy every libuv threadpool thread in this worker. */ -async function mapWithConcurrency( - items: readonly T[], - limit: number, - mapper: (item: T) => Promise -): Promise { - const results = Array.from({ length: items.length }) - let cursor = 0 - const worker = async (): Promise => { - while (cursor < items.length) { - const index = cursor++ - results[index] = await mapper(items[index]) - } - } - await Promise.all(Array.from({ length: Math.min(limit, items.length) }, worker)) - return results -} - -async function main(): Promise { - let watcher: typeof ParcelWatcher - try { - watcher = await import('@parcel/watcher') - } catch (err) { - port.postMessage({ - type: 'error', - message: err instanceof Error ? err.message : String(err) - } satisfies FileWatcherWorkerMessage) - return - } - - const subscription = await watcher.subscribe( - data.rootPath, - (err, events) => { - if (err) { - reportWatchError(err) - return - } - // Why: large watcher batches usually mean a generated directory or branch - // switch. Avoid stat fanout and ask the renderer to refresh. - if (events.length > RUNTIME_FILE_WATCH_EVENT_STAT_LIMIT) { - port.postMessage({ - type: 'events', - events: [{ kind: 'overflow', absolutePath: data.rootPath }] - } satisfies FileWatcherWorkerMessage) - return - } - void mapWithConcurrency( - events, - RUNTIME_FILE_WATCH_STAT_CONCURRENCY, - async (event): Promise => { - let isDirectory = false - try { - isDirectory = (await stat(event.path)).isDirectory() - } catch { - isDirectory = false - } - return { kind: event.type, absolutePath: event.path, isDirectory } - } - ) - .then((mapped) => { - port.postMessage({ type: 'events', events: mapped } satisfies FileWatcherWorkerMessage) - }) - // Why: without this, a throwing postMessage / stat becomes an unhandled - // rejection that crashes the worker silently. Surface it instead. - .catch((err: unknown) => reportWatchError(err)) - }, - data.ignoreOptions as ParcelWatcher.Options - ) - - // The crawl finished and the subscription is live. - port.postMessage({ type: 'ready' } satisfies FileWatcherWorkerMessage) - - port.on('message', (message: FileWatcherHostMessage) => { - if (message.type === 'unsubscribe') { - void subscription.unsubscribe().finally(() => { - port.close() - }) - } - }) -} - -void main().catch((err: unknown) => { - port.postMessage({ - type: 'error', - message: err instanceof Error ? err.message : String(err) - } satisfies FileWatcherWorkerMessage) -}) diff --git a/src/main/runtime/orca-runtime-files-watch.test.ts b/src/main/runtime/orca-runtime-files-watch.test.ts index 014b7b8e774..7986e0fc9b0 100644 --- a/src/main/runtime/orca-runtime-files-watch.test.ts +++ b/src/main/runtime/orca-runtime-files-watch.test.ts @@ -4,11 +4,18 @@ import type * as FsPromises from 'node:fs/promises' import type * as FilesystemAuth from '../ipc/filesystem-auth' import type { FsChangeEvent } from '../../shared/types' -const { resolveAuthorizedPathMock, statMock, watchMock, watchInWorkerMock } = vi.hoisted(() => ({ +const { + resolveAuthorizedPathMock, + statMock, + watchMock, + watchInWatcherProcessMock, + getSshFilesystemProviderMock +} = vi.hoisted(() => ({ resolveAuthorizedPathMock: vi.fn(), statMock: vi.fn(), watchMock: vi.fn(), - watchInWorkerMock: vi.fn() + watchInWatcherProcessMock: vi.fn(), + getSshFilesystemProviderMock: vi.fn() })) vi.mock('fs', async () => { @@ -27,9 +34,9 @@ vi.mock('fs/promises', async () => { } }) -// The local (non-Windows, non-SSH) watch path now delegates to a worker thread. +// The local (non-Windows, non-SSH) watch path delegates to the isolated watcher process. vi.mock('./file-watcher-host', () => ({ - watchFileExplorerInWorker: watchInWorkerMock + watchFileExplorerInWatcherProcess: watchInWatcherProcessMock })) vi.mock('../ipc/filesystem-auth', async () => { @@ -40,6 +47,10 @@ vi.mock('../ipc/filesystem-auth', async () => { } }) +vi.mock('../providers/ssh-filesystem-dispatch', () => ({ + getSshFilesystemProvider: getSshFilesystemProviderMock +})) + import { awaitRuntimeFileWatcherUnsubscribes, RuntimeFileCommands } from './orca-runtime-files' function createRuntimeFileCommands(rootPath: string) { @@ -67,15 +78,16 @@ function createRuntimeFileCommands(rootPath: string) { describe('RuntimeFileCommands file watching', () => { const originalPlatform = process.platform - // Why: Windows runtime watches intentionally use fs.watch instead of the worker path. - const posixWorkerIt = process.platform === 'win32' ? it.skip : it + // Why: Windows runtime watches intentionally use fs.watch instead of the watcher child. + const posixWatcherProcessIt = process.platform === 'win32' ? it.skip : it beforeEach(() => { vi.useFakeTimers() resolveAuthorizedPathMock.mockReset() statMock.mockReset() watchMock.mockReset() - watchInWorkerMock.mockReset() + watchInWatcherProcessMock.mockReset() + getSshFilesystemProviderMock.mockReset() Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform @@ -112,8 +124,8 @@ describe('RuntimeFileCommands file watching', () => { const unsubscribe = await commands.watchFileExplorer('id:wt-1', onEvents) expect(watchMock).toHaveBeenCalledWith('C:\\repo', { recursive: true }, expect.any(Function)) - // Windows path does not go through the worker. - expect(watchInWorkerMock).not.toHaveBeenCalled() + // Windows path does not go through the watcher child. + expect(watchInWatcherProcessMock).not.toHaveBeenCalled() const emit = listener as (() => void) | null expect(emit).not.toBeNull() @@ -130,48 +142,61 @@ describe('RuntimeFileCommands file watching', () => { expect(close).toHaveBeenCalledTimes(1) }) - // Issue #5308: the local recursive watch runs in a worker thread so - // @parcel/watcher's blocking initial crawl can't starve the serve runtime. - posixWorkerIt('delegates local recursive watching to the worker thread', async () => { + // Issues #5308/#8212: the local recursive watch runs out of process so the + // blocking initial crawl and native faults cannot take down the serve runtime. + posixWatcherProcessIt('delegates local recursive watching to the watcher process', async () => { resolveAuthorizedPathMock.mockResolvedValue('/home5/Brian') statMock.mockResolvedValue({ isDirectory: () => true }) const captured: { cb?: (events: FsChangeEvent[]) => void } = {} - const workerDispose = vi.fn() - watchInWorkerMock.mockImplementation((_rootPath, cb) => { + const watcherDispose = vi.fn() + watchInWatcherProcessMock.mockImplementation((_rootPath, cb) => { captured.cb = cb - return Promise.resolve(workerDispose) + return Promise.resolve(watcherDispose) }) const onEvents = vi.fn() const { commands } = createRuntimeFileCommands('/home5/Brian') - const unsubscribe = await commands.watchFileExplorer('id:wt-1', onEvents) + const controller = new AbortController() + const unsubscribe = await commands.watchFileExplorer( + 'id:wt-1', + onEvents, + vi.fn(), + controller.signal + ) - expect(watchInWorkerMock).toHaveBeenCalledWith('/home5/Brian', expect.any(Function)) + expect(watchInWatcherProcessMock).toHaveBeenCalledWith( + '/home5/Brian', + expect.any(Function), + expect.any(Function), + controller.signal + ) - // Events surfaced by the worker reach the caller. + // Events surfaced by the watcher process reach the caller. captured.cb?.([{ kind: 'update', absolutePath: '/home5/Brian/a.txt', isDirectory: false }]) expect(onEvents).toHaveBeenCalledWith([ { kind: 'update', absolutePath: '/home5/Brian/a.txt', isDirectory: false } ]) - // Unsubscribe tears the worker down (dispose runs on the shutdown-drain + // Unsubscribe tears the subscription down (dispose runs on the shutdown-drain // microtask, so await the drain before asserting). unsubscribe() await awaitRuntimeFileWatcherUnsubscribes() - expect(workerDispose).toHaveBeenCalledTimes(1) + expect(watcherDispose).toHaveBeenCalledTimes(1) }) - posixWorkerIt('propagates a worker watch failure to the caller', async () => { + posixWatcherProcessIt('propagates a watcher process failure to the caller', async () => { resolveAuthorizedPathMock.mockResolvedValue('/repo') statMock.mockResolvedValue({ isDirectory: () => true }) - watchInWorkerMock.mockRejectedValue(new Error('worker_failed')) + watchInWatcherProcessMock.mockRejectedValue(new Error('watcher_process_failed')) const { commands } = createRuntimeFileCommands('/repo') - await expect(commands.watchFileExplorer('id:wt-1', vi.fn())).rejects.toThrow('worker_failed') + await expect(commands.watchFileExplorer('id:wt-1', vi.fn())).rejects.toThrow( + 'watcher_process_failed' + ) }) - posixWorkerIt('tracks worker unsubscribe work so shutdown can await it', async () => { + posixWatcherProcessIt('tracks watcher unsubscribe work so shutdown can await it', async () => { resolveAuthorizedPathMock.mockResolvedValue('/repo') statMock.mockResolvedValue({ isDirectory: () => true }) @@ -182,7 +207,7 @@ describe('RuntimeFileCommands file watching', () => { resolveDispose = resolve }) ) - watchInWorkerMock.mockResolvedValue(disposeMock) + watchInWatcherProcessMock.mockResolvedValue(disposeMock) const { commands } = createRuntimeFileCommands('/repo') const unsubscribe = await commands.watchFileExplorer('id:wt-1', vi.fn()) @@ -201,4 +226,37 @@ describe('RuntimeFileCommands file watching', () => { await drainPromise expect(drained).toBe(true) }) + + it('forwards the abort signal into SSH-backed file explorer watches', async () => { + const watch = vi.fn(async () => () => {}) + getSshFilesystemProviderMock.mockReturnValue({ watch }) + const store = { getRepo: vi.fn(() => ({ connectionId: 'ssh-1' })) } + const commands = new RuntimeFileCommands({ + getRuntimeId: () => 'runtime-1', + requireStore: () => store, + resolveWorktreeSelector: vi.fn(async () => ({ + id: 'wt-1', + repoId: 'repo-1', + path: '/remote/repo' + })), + resolveRuntimeFileTarget: vi.fn(async () => ({ + worktree: { + id: 'wt-1', + repoId: 'repo-1', + path: '/remote/repo' + }, + connectionId: 'ssh-1' + })), + resolveRuntimeGitTarget: vi.fn(), + openFile: vi.fn() + } as never) + const controller = new AbortController() + + await commands.watchFileExplorer('id:wt-1', vi.fn(), vi.fn(), controller.signal) + + expect(watch).toHaveBeenCalledWith('/remote/repo', expect.any(Function), { + signal: controller.signal + }) + expect(watchInWatcherProcessMock).not.toHaveBeenCalled() + }) }) diff --git a/src/main/runtime/orca-runtime-files.test.ts b/src/main/runtime/orca-runtime-files.test.ts index 8412a7740be..d43e48c4695 100644 --- a/src/main/runtime/orca-runtime-files.test.ts +++ b/src/main/runtime/orca-runtime-files.test.ts @@ -18,7 +18,7 @@ const { renameMock, resolveAuthorizedPathMock, statMock, - watchInWorkerMock, + watchInWatcherProcessMock, checkRgAvailableMock, getLocalGitOptionsForRegisteredWorktreeMock, wslAwareSpawnMock, @@ -32,7 +32,7 @@ const { renameMock: vi.fn(), resolveAuthorizedPathMock: vi.fn(), statMock: vi.fn(), - watchInWorkerMock: vi.fn(), + watchInWatcherProcessMock: vi.fn(), wslAwareSpawnMock: vi.fn(), watchMock: vi.fn() })) @@ -61,7 +61,7 @@ vi.mock('fs/promises', async () => { }) vi.mock('./file-watcher-host', () => ({ - watchFileExplorerInWorker: watchInWorkerMock + watchFileExplorerInWatcherProcess: watchInWatcherProcessMock })) vi.mock('../ipc/filesystem-auth', async () => { @@ -199,7 +199,7 @@ describe('RuntimeFileCommands', () => { renameMock.mockReset() resolveAuthorizedPathMock.mockReset() statMock.mockReset() - watchInWorkerMock.mockReset() + watchInWatcherProcessMock.mockReset() watchMock.mockReset() checkRgAvailableMock.mockReset() vi.mocked(getSshFilesystemProvider).mockReset() @@ -441,7 +441,7 @@ describe('RuntimeFileCommands', () => { expect(close).toHaveBeenCalledTimes(1) }) - it('delegates local recursive watching to the worker thread', async () => { + it('delegates local recursive watching to the watcher process', async () => { Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' @@ -449,11 +449,16 @@ describe('RuntimeFileCommands', () => { resolveAuthorizedPathMock.mockResolvedValue('/repo') statMock.mockResolvedValue({ isDirectory: () => true }) const dispose = vi.fn() - watchInWorkerMock.mockResolvedValue(dispose) + watchInWatcherProcessMock.mockResolvedValue(dispose) const { commands } = createRuntimeFileCommands() const unsubscribe = await commands.watchFileExplorer('id:wt-1', vi.fn()) - expect(watchInWorkerMock).toHaveBeenCalledWith('/repo', expect.any(Function)) + expect(watchInWatcherProcessMock).toHaveBeenCalledWith( + '/repo', + expect.any(Function), + expect.any(Function), + undefined + ) unsubscribe() await awaitRuntimeFileWatcherUnsubscribes() diff --git a/src/main/runtime/orca-runtime-files.ts b/src/main/runtime/orca-runtime-files.ts index e8a74e3591c..acdf3616f47 100644 --- a/src/main/runtime/orca-runtime-files.ts +++ b/src/main/runtime/orca-runtime-files.ts @@ -44,7 +44,7 @@ import type { RuntimeFileReadResult, RuntimeTerminalPathResolution } from '../../shared/runtime-types' -import { watchFileExplorerInWorker } from './file-watcher-host' +import { watchFileExplorerInWatcherProcess } from './file-watcher-host' import { wslAwareSpawn } from '../git/runner' import { parseWslPath, toWindowsWslPath } from '../wsl' import { isENOENT, resolveAuthorizedPath } from '../ipc/filesystem-auth' @@ -921,7 +921,9 @@ export class RuntimeFileCommands { async watchFileExplorer( worktreeSelector: string, - callback: (events: FsChangeEvent[]) => void + callback: (events: FsChangeEvent[]) => void, + onTerminalError: (error: Error) => void = () => undefined, + signal?: AbortSignal ): Promise<() => void> { const target = await this.resolveFileExplorerPath(worktreeSelector, '') const provider = target.connectionId ? getSshFilesystemProvider(target.connectionId) : null @@ -929,7 +931,9 @@ export class RuntimeFileCommands { if (!provider) { throw new Error(SSH_FILESYSTEM_PROVIDER_UNAVAILABLE_MESSAGE) } - return provider.watch(target.path, callback) + // Why: the RPC layer already threads AbortSignal for local watches; SSH + // must cancel the remote fs.watch request instead of waiting it out. + return provider.watch(target.path, callback, { signal }) } const rootPath = await resolveAuthorizedPath(target.path, this.host.requireStore()) @@ -940,9 +944,14 @@ export class RuntimeFileCommands { if (process.platform === 'win32') { return watchWindowsRuntimeFileExplorer(rootPath, callback) } - // Why: the watcher runs in a worker thread so @parcel/watcher's blocking - // recursive crawl can't starve the main/`serve` process (issue #5308). - const dispose = await watchFileExplorerInWorker(rootPath, callback) + // Why: the forked watcher keeps the blocking crawl and native faults out + // of the main/`serve` process (issues #5308 and #8212). + const dispose = await watchFileExplorerInWatcherProcess( + rootPath, + callback, + onTerminalError, + signal + ) return () => { trackRuntimeFileWatcherUnsubscribe(rootPath, dispose) } diff --git a/src/main/runtime/rpc/methods/files.test.ts b/src/main/runtime/rpc/methods/files.test.ts index 7282489c706..9f535d9a637 100644 --- a/src/main/runtime/rpc/methods/files.test.ts +++ b/src/main/runtime/rpc/methods/files.test.ts @@ -128,7 +128,12 @@ describe('file RPC methods', () => { await vi.waitFor(() => { expect(replies).toHaveLength(1) }) - expect(runtime.watchFileExplorer).toHaveBeenCalledWith('id:wt-1', expect.any(Function)) + expect(runtime.watchFileExplorer).toHaveBeenCalledWith( + 'id:wt-1', + expect.any(Function), + expect.any(Function), + undefined + ) expect(replies[0]).toMatchObject({ ok: true, streaming: true, @@ -172,17 +177,63 @@ describe('file RPC methods', () => { } }) + it('ends a ready file-watch stream when its watcher fails terminally', async () => { + let onTerminalError: ((error: Error) => void) | undefined + const unwatch = vi.fn() + const cleanups = new Map void>() + let emitWatchChange: + | ((events: { kind: 'overflow'; absolutePath: string }[]) => void) + | undefined + const runtime = { + getRuntimeId: () => 'test-runtime', + watchFileExplorer: vi.fn(async (_worktree, callback, nextTerminalError) => { + emitWatchChange = callback + onTerminalError = nextTerminalError + return unwatch + }), + registerSubscriptionCleanup: vi.fn((id, cleanup) => cleanups.set(id, cleanup)), + cleanupSubscription: vi.fn((id) => { + const cleanup = cleanups.get(id) + cleanups.delete(id) + cleanup?.() + }) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: FILE_METHODS }) + const replies: { result?: { type?: string; message?: string } }[] = [] + + const dispatch = dispatcher.dispatchStreaming( + makeRequest('files.watch', { worktree: 'id:wt-1' }), + (response) => replies.push(JSON.parse(response)) + ) + await vi.waitFor(() => expect(replies[0]?.result?.type).toBe('ready')) + + emitWatchChange?.([{ kind: 'overflow', absolutePath: '/repo' }]) + onTerminalError?.(new Error('file watcher process crashed repeatedly')) + await dispatch + + expect(replies.map((reply) => reply.result?.type)).toEqual(['ready', 'changed', 'error', 'end']) + expect(replies[2]?.result?.message).toContain('crashed repeatedly') + expect(unwatch).toHaveBeenCalledTimes(1) + expect(cleanups.size).toBe(0) + }) + it('tears down a file watch that resolves after the connection already closed', async () => { type WatchCallback = ( events: { kind: 'update'; absolutePath: string; isDirectory?: boolean }[] ) => void const unwatch = vi.fn() let resolveWatch: (value: () => void) => void = () => {} - const watchFileExplorer = vi.fn((_worktree: string, _callback: WatchCallback) => { - return new Promise<() => void>((resolve) => { - resolveWatch = resolve - }) - }) + const watchFileExplorer = vi.fn( + ( + _worktree: string, + _callback: WatchCallback, + _onTerminalError?: (error: Error) => void, + _signal?: AbortSignal + ) => + new Promise<() => void>((resolve) => { + resolveWatch = resolve + }) + ) const runtime = { getRuntimeId: () => 'test-runtime', watchFileExplorer, @@ -200,6 +251,7 @@ describe('file RPC methods', () => { await vi.waitFor(() => { expect(watchFileExplorer).toHaveBeenCalled() }) + expect(watchFileExplorer.mock.calls[0]?.[3]).toBe(abortController.signal) abortController.abort() await dispatch diff --git a/src/main/runtime/rpc/methods/files.ts b/src/main/runtime/rpc/methods/files.ts index 37224f18ed5..b1fb70234a6 100644 --- a/src/main/runtime/rpc/methods/files.ts +++ b/src/main/runtime/rpc/methods/files.ts @@ -392,6 +392,7 @@ export const FILE_METHODS: RpcAnyMethod[] = [ await new Promise((resolve, reject) => { let settled = false let unwatch: (() => void) | null = null + let terminalError: Error | null = null const eventBatcher = createFileWatchEventBatcher(params.worktree, emit) const finish = (): void => { if (settled) { @@ -402,11 +403,32 @@ export const FILE_METHODS: RpcAnyMethod[] = [ resolve() } const cleanup = (): void => { - eventBatcher.flush() - eventBatcher.dispose() + // Mark cleanup complete before emitting so a synchronous transport + // abort cannot re-enter this function and duplicate error/end. + finish() + if (terminalError) { + // Why: recovery emits overflow before giving up. Flush that final + // refresh so clients never end on a knowingly stale snapshot. + eventBatcher.flush() + eventBatcher.dispose() + emit({ type: 'error', message: terminalError.message }) + } else { + eventBatcher.flush() + eventBatcher.dispose() + } unwatch?.() emit({ type: 'end' }) - finish() + } + const handleTerminalError = (error: Error): void => { + if (settled || terminalError) { + return + } + terminalError = error + if (unwatch) { + runtime.cleanupSubscription(subscriptionId) + } else { + cleanup() + } } const handleAbort = (): void => { if (unwatch) { @@ -420,9 +442,14 @@ export const FILE_METHODS: RpcAnyMethod[] = [ } signal?.addEventListener('abort', handleAbort, { once: true }) void runtime - .watchFileExplorer(params.worktree, (events) => { - eventBatcher.push(events) - }) + .watchFileExplorer( + params.worktree, + (events) => { + eventBatcher.push(events) + }, + handleTerminalError, + signal + ) .then((nextUnwatch) => { if (signal?.aborted || settled) { // Why: the connection can close while watch setup is still @@ -438,7 +465,9 @@ export const FILE_METHODS: RpcAnyMethod[] = [ }) .catch((error) => { if (!settled) { + settled = true signal?.removeEventListener('abort', handleAbort) + eventBatcher.dispose() reject(error) } }) diff --git a/src/renderer/src/runtime/runtime-file-client.test.ts b/src/renderer/src/runtime/runtime-file-client.test.ts index 7b80f59f5b9..52086493559 100644 --- a/src/renderer/src/runtime/runtime-file-client.test.ts +++ b/src/renderer/src/runtime/runtime-file-client.test.ts @@ -1736,6 +1736,115 @@ describe('runtime file client', () => { expect(unsubscribe).toHaveBeenCalled() }) + it('surfaces terminal watcher errors and allows a fresh subscription after stream end', async () => { + const onError = vi.fn() + const unsubscribe = vi.fn() + let callbacks: { onResponse: (response: unknown) => void; onClose: () => void } | undefined + runtimeEnvironmentSubscribe.mockImplementation((_args, nextCallbacks) => { + callbacks = nextCallbacks + return Promise.resolve({ unsubscribe, sendBinary: vi.fn() }) + }) + + await subscribeRuntimeFileChanges( + { + settings: { activeRuntimeEnvironmentId: 'env-1' }, + worktreeId: 'wt-1', + worktreePath: '/remote/repo' + }, + vi.fn(), + onError + ) + callbacks?.onResponse({ + id: 'rpc-error', + ok: true, + result: { type: 'error', message: 'file watcher process crashed repeatedly' }, + _meta: { runtimeId: 'remote-runtime' } + }) + callbacks?.onResponse({ + id: 'rpc-end', + ok: true, + result: { type: 'end' }, + _meta: { runtimeId: 'remote-runtime' } + }) + + expect(onError).toHaveBeenCalledWith( + expect.objectContaining({ message: 'file watcher process crashed repeatedly' }) + ) + expect(unsubscribe).toHaveBeenCalledTimes(1) + + await subscribeRuntimeFileChanges( + { + settings: { activeRuntimeEnvironmentId: 'env-1' }, + worktreeId: 'wt-1', + worktreePath: '/remote/repo' + }, + vi.fn(), + vi.fn() + ) + expect(runtimeEnvironmentSubscribe).toHaveBeenCalledTimes(2) + callbacks?.onResponse({ + id: 'rpc-second-end', + ok: true, + result: { type: 'end' }, + _meta: { runtimeId: 'remote-runtime' } + }) + }) + + it('evicts a terminal watch before notifying error listeners that retry', async () => { + const callbacks: { + onResponse: (response: unknown) => void + onClose: () => void + }[] = [] + runtimeEnvironmentSubscribe.mockImplementation((_args, nextCallbacks) => { + callbacks.push(nextCallbacks) + return Promise.resolve({ unsubscribe: vi.fn(), sendBinary: vi.fn() }) + }) + const retryPayload = vi.fn() + let retryPromise: Promise<() => void> | undefined + const context = { + settings: { activeRuntimeEnvironmentId: 'env-1' }, + worktreeId: 'wt-1', + worktreePath: '/remote/repo' + } + await subscribeRuntimeFileChanges(context, vi.fn(), () => { + retryPromise = subscribeRuntimeFileChanges(context, retryPayload, vi.fn()) + }) + + callbacks[0].onResponse({ + id: 'rpc-error', + ok: true, + result: { type: 'error', message: 'terminal watcher failure' }, + _meta: { runtimeId: 'remote-runtime' } + }) + await retryPromise + const subscribeCallCount = runtimeEnvironmentSubscribe.mock.calls.length + callbacks[0].onResponse({ + id: 'rpc-end', + ok: true, + result: { type: 'end' }, + _meta: { runtimeId: 'remote-runtime' } + }) + callbacks[1]?.onResponse({ + id: 'rpc-changed', + ok: true, + result: { + type: 'changed', + worktree: 'id:wt-1', + events: [{ kind: 'update', absolutePath: '/remote/repo/retry.txt' }] + }, + _meta: { runtimeId: 'remote-runtime' } + }) + callbacks[1]?.onResponse({ + id: 'rpc-retry-end', + ok: true, + result: { type: 'end' }, + _meta: { runtimeId: 'remote-runtime' } + }) + + expect(subscribeCallCount).toBe(2) + expect(retryPayload).toHaveBeenCalledTimes(1) + }) + it('shares one remote file watch subscription across listeners for the same worktree', async () => { const firstPayload = vi.fn() const secondPayload = vi.fn() diff --git a/src/renderer/src/runtime/runtime-file-client.ts b/src/renderer/src/runtime/runtime-file-client.ts index 3265ad343b0..6c70cd9e63c 100644 --- a/src/renderer/src/runtime/runtime-file-client.ts +++ b/src/renderer/src/runtime/runtime-file-client.ts @@ -101,6 +101,7 @@ type RuntimeImportResult = type RuntimeFileWatchEvent = | { type: 'ready'; subscriptionId: string } | { type: 'changed'; worktree: string; events: FsChangedPayload['events'] } + | { type: 'error'; message: string } | { type: 'end' } const REMOTE_UPLOAD_BASE64_CHUNK_CHARS = 512 * 1024 @@ -886,7 +887,7 @@ function createSharedRuntimeFileWatch( }, { onResponse: (response) => { - handleSharedRuntimeFileWatchResponse(shared, worktreePath, response) + handleSharedRuntimeFileWatchResponse(key, shared, worktreePath, response) }, onError: (error) => { notifySharedRuntimeFileWatchError(shared, new Error(error.message)) @@ -922,6 +923,7 @@ function createSharedRuntimeFileWatch( } function handleSharedRuntimeFileWatchResponse( + key: string, shared: SharedRuntimeFileWatch, worktreePath: string, response: unknown @@ -943,6 +945,30 @@ function handleSharedRuntimeFileWatchResponse( for (const listener of Array.from(shared.listeners)) { listener.onPayload({ worktreePath, events: event.events }) } + } else if (event.type === 'error') { + // Why: error listeners may synchronously retry. Evict the terminal watch + // before callbacks run so the retry cannot join a stream awaiting `end`. + if (sharedRuntimeFileWatches.get(key) === shared) { + sharedRuntimeFileWatches.delete(key) + } + shared.closed = true + const listeners = Array.from(shared.listeners) + shared.listeners.clear() + for (const listener of listeners) { + listener.onError?.(new Error(event.message)) + } + } else if (event.type === 'end') { + // Why: shared-control completes without onClose; evict and release its + // transport handle so later listeners start cleanly without retained state. + if (sharedRuntimeFileWatches.get(key) === shared) { + sharedRuntimeFileWatches.delete(key) + } + shared.closed = true + const unsubscribe = shared.unsubscribe + shared.unsubscribe = null + shared.remoteSubscriptionId = null + shared.listeners.clear() + unsubscribe?.() } } catch (err) { notifySharedRuntimeFileWatchError(shared, err instanceof Error ? err : new Error(String(err))) From 4a76a1c894bf8f7258b05f56ae1bebac43e273c0 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 13 Jul 2026 01:19:26 +0000 Subject: [PATCH 4/4] release: v1.4.138-rc.4 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 2b51abd8e51..e361e666045 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "orca", - "version": "1.4.138-rc.3", + "version": "1.4.138-rc.4", "description": "Next-gen IDE for parallel agentic development", "homepage": "https://github.com/stablyai/orca", "author": "stablyai",