diff --git a/.storybook/mocks/orpc.ts b/.storybook/mocks/orpc.ts index dec1c2a70f..a5184edd3f 100644 --- a/.storybook/mocks/orpc.ts +++ b/.storybook/mocks/orpc.ts @@ -27,6 +27,19 @@ export interface MockORPCClientOptions { providersList?: string[]; /** Mock for projects.remove - return error string to simulate failure */ onProjectRemove?: (projectPath: string) => { success: true } | { success: false; error: string }; + /** Background processes per workspace */ + backgroundProcesses?: Map< + string, + Array<{ + id: string; + pid: number; + script: string; + displayName?: string; + startTime: number; + status: "running" | "exited" | "killed" | "failed"; + exitCode?: number; + }> + >; } /** @@ -55,6 +68,7 @@ export function createMockORPCClient(options: MockORPCClientOptions = {}): APICl providersConfig = {}, providersList = [], onProjectRemove, + backgroundProcesses = new Map(), } = options; const workspaceMap = new Map(workspaces.map((w) => [w.id, w])); @@ -178,6 +192,19 @@ export function createMockORPCClient(options: MockORPCClientOptions = {}): APICl await new Promise(() => {}); // Never resolves }, }, + backgroundBashes: { + subscribe: async function* (input: { workspaceId: string }) { + // Yield initial state + yield { + processes: backgroundProcesses.get(input.workspaceId) ?? [], + foregroundToolCallIds: [], + }; + // Then hang forever (like a real subscription) + await new Promise(() => {}); + }, + terminate: async () => ({ success: true, data: undefined }), + sendToBackground: async () => ({ success: true, data: undefined }), + }, }, window: { setTitle: async () => undefined, diff --git a/src/browser/components/AIView.tsx b/src/browser/components/AIView.tsx index b31a5f063d..42741afd62 100644 --- a/src/browser/components/AIView.tsx +++ b/src/browser/components/AIView.tsx @@ -51,6 +51,8 @@ import { evictModelFromLRU } from "@/browser/hooks/useModelLRU"; import { QueuedMessage } from "./Messages/QueuedMessage"; import { CompactionWarning } from "./CompactionWarning"; import { ConcurrentLocalWarning } from "./ConcurrentLocalWarning"; +import { BackgroundProcessesBanner } from "./BackgroundProcessesBanner"; +import { useBackgroundBashHandlers } from "@/browser/hooks/useBackgroundBashHandlers"; import { checkAutoCompaction } from "@/browser/utils/compaction/autoCompactionCheck"; import { executeCompaction } from "@/browser/utils/chatCommands"; import { useProviderOptions } from "@/browser/hooks/useProviderOptions"; @@ -61,6 +63,7 @@ import { useAPI } from "@/browser/contexts/API"; import { useReviews } from "@/browser/hooks/useReviews"; import { ReviewsBanner } from "./ReviewsBanner"; import type { ReviewNoteData } from "@/common/types/review"; +import { PopoverError } from "./PopoverError"; interface AIViewProps { workspaceId: string; @@ -119,6 +122,15 @@ const AIViewInner: React.FC = ({ // Reviews state const reviews = useReviews(workspaceId); + + const { + processes: backgroundBashes, + handleTerminate: handleTerminateBackgroundBash, + foregroundToolCallIds, + handleSendToBackground: handleSendBashToBackground, + handleMessageSentBackground, + error: backgroundBashError, + } = useBackgroundBashHandlers(api, workspaceId); const { options } = useProviderOptions(); const use1M = options.anthropic?.use1MContext ?? false; // Get pending model for auto-compaction settings (threshold is per-model) @@ -312,13 +324,17 @@ const AIViewInner: React.FC = ({ }, []); const handleMessageSent = useCallback(() => { + // Auto-background any running foreground bash when user sends a new message + // This prevents the user from waiting for the bash to complete before their message is processed + handleMessageSentBackground(); + // Enable auto-scroll when user sends a message setAutoScroll(true); // Reset autoRetry when user sends a message // User action = clear intent: "I'm actively using this workspace" setAutoRetry(true); - }, [setAutoScroll, setAutoRetry]); + }, [setAutoScroll, setAutoRetry, handleMessageSentBackground]); const handleClearHistory = useCallback( async (percentage = 1.0) => { @@ -573,6 +589,8 @@ const AIViewInner: React.FC = ({ msg.toolName === "propose_plan" && msg.id === latestProposePlanId } + foregroundBashToolCallIds={foregroundToolCallIds} + onSendBashToBackground={handleSendBashToBackground} /> {isAtCutoff && ( @@ -647,6 +665,10 @@ const AIViewInner: React.FC = ({ onCompactClick={handleCompactClick} /> )} + = ({ onReviewNote={handleReviewNote} // Pass review note handler to append to chat isCreating={status === "creating"} // Workspace still being set up /> + + ); }; diff --git a/src/browser/components/BackgroundProcessesBanner.tsx b/src/browser/components/BackgroundProcessesBanner.tsx new file mode 100644 index 0000000000..bad90f8445 --- /dev/null +++ b/src/browser/components/BackgroundProcessesBanner.tsx @@ -0,0 +1,125 @@ +import React, { useState, useCallback, useEffect } from "react"; +import { Terminal, X, ChevronDown, ChevronRight } from "lucide-react"; +import { Tooltip, TooltipTrigger, TooltipContent } from "./ui/tooltip"; +import type { BackgroundProcessInfo } from "@/common/orpc/schemas/api"; +import { cn } from "@/common/lib/utils"; +import { formatDuration } from "./tools/shared/toolUtils"; + +/** + * Truncate script to reasonable display length. + */ +function truncateScript(script: string, maxLength = 60): string { + // First line only, truncated + const firstLine = script.split("\n")[0] ?? script; + if (firstLine.length <= maxLength) { + return firstLine; + } + return firstLine.slice(0, maxLength - 3) + "..."; +} + +interface BackgroundProcessesBannerProps { + processes: BackgroundProcessInfo[]; + onTerminate: (processId: string) => void; +} + +/** + * Banner showing running background processes. + * Displays "N running bashes" which expands on click to show details. + */ +export const BackgroundProcessesBanner: React.FC = (props) => { + const [isExpanded, setIsExpanded] = useState(false); + const [, setTick] = useState(0); + + // Filter to only running processes + const runningProcesses = props.processes.filter((p) => p.status === "running"); + const count = runningProcesses.length; + + // Update duration display every second when expanded + useEffect(() => { + if (!isExpanded || count === 0) return; + const interval = setInterval(() => setTick((t) => t + 1), 1000); + return () => clearInterval(interval); + }, [isExpanded, count]); + + const { onTerminate } = props; + const handleTerminate = useCallback( + (processId: string, event: React.MouseEvent) => { + event.stopPropagation(); + onTerminate(processId); + }, + [onTerminate] + ); + + const handleToggle = useCallback(() => { + setIsExpanded((prev) => !prev); + }, []); + + // Don't render if no running processes + if (count === 0) { + return null; + } + + return ( +
+ {/* Collapsed banner - thin stripe, content aligned with chat */} + + + {/* Expanded view - content aligned with chat */} + {isExpanded && ( +
+ {runningProcesses.map((proc) => ( +
+
+
+ {proc.displayName ?? truncateScript(proc.script)} +
+
pid {proc.pid}
+
+
+ + {formatDuration(Date.now() - proc.startTime)} + + + + + + Terminate process + +
+
+ ))} +
+ )} +
+ ); +}; diff --git a/src/browser/components/Messages/MessageRenderer.tsx b/src/browser/components/Messages/MessageRenderer.tsx index c13adc8e18..8e3e29425d 100644 --- a/src/browser/components/Messages/MessageRenderer.tsx +++ b/src/browser/components/Messages/MessageRenderer.tsx @@ -22,6 +22,10 @@ interface MessageRendererProps { onReviewNote?: (data: ReviewNoteData) => void; /** Whether this message is the latest propose_plan tool call (for external edit detection) */ isLatestProposePlan?: boolean; + /** Set of tool call IDs of foreground bashes */ + foregroundBashToolCallIds?: Set; + /** Callback to send a foreground bash to background */ + onSendBashToBackground?: (toolCallId: string) => void; } // Memoized to prevent unnecessary re-renders when parent (AIView) updates @@ -34,6 +38,8 @@ export const MessageRenderer = React.memo( isCompacting, onReviewNote, isLatestProposePlan, + foregroundBashToolCallIds, + onSendBashToBackground, }) => { // Route based on message type switch (message.type) { @@ -63,6 +69,8 @@ export const MessageRenderer = React.memo( workspaceId={workspaceId} onReviewNote={onReviewNote} isLatestProposePlan={isLatestProposePlan} + foregroundBashToolCallIds={foregroundBashToolCallIds} + onSendBashToBackground={onSendBashToBackground} /> ); case "reasoning": diff --git a/src/browser/components/Messages/ToolMessage.tsx b/src/browser/components/Messages/ToolMessage.tsx index 319bb6e93f..b0356f2461 100644 --- a/src/browser/components/Messages/ToolMessage.tsx +++ b/src/browser/components/Messages/ToolMessage.tsx @@ -11,6 +11,7 @@ import { StatusSetToolCall } from "../tools/StatusSetToolCall"; import { WebFetchToolCall } from "../tools/WebFetchToolCall"; import { BashBackgroundListToolCall } from "../tools/BashBackgroundListToolCall"; import { BashBackgroundTerminateToolCall } from "../tools/BashBackgroundTerminateToolCall"; +import { BashOutputToolCall } from "../tools/BashOutputToolCall"; import type { BashToolArgs, BashToolResult, @@ -18,6 +19,8 @@ import type { BashBackgroundListResult, BashBackgroundTerminateArgs, BashBackgroundTerminateResult, + BashOutputToolArgs, + BashOutputToolResult, FileReadToolArgs, FileReadToolResult, FileEditReplaceStringToolArgs, @@ -45,6 +48,10 @@ interface ToolMessageProps { onReviewNote?: (data: ReviewNoteData) => void; /** Whether this is the latest propose_plan in the conversation */ isLatestProposePlan?: boolean; + /** Set of tool call IDs of foreground bashes */ + foregroundBashToolCallIds?: Set; + /** Callback to send a foreground bash to background */ + onSendBashToBackground?: (toolCallId: string) => void; } // Type guards using Zod schemas for single source of truth @@ -113,15 +120,25 @@ function isBashBackgroundTerminateTool( return TOOL_DEFINITIONS.bash_background_terminate.schema.safeParse(args).success; } +function isBashOutputTool(toolName: string, args: unknown): args is BashOutputToolArgs { + if (toolName !== "bash_output") return false; + return TOOL_DEFINITIONS.bash_output.schema.safeParse(args).success; +} + export const ToolMessage: React.FC = ({ message, className, workspaceId, onReviewNote, isLatestProposePlan, + foregroundBashToolCallIds, + onSendBashToBackground, }) => { // Route to specialized components based on tool name if (isBashTool(message.toolName, message.args)) { + // Only show "Background" button if this specific tool call is a foreground process + const canSendToBackground = foregroundBashToolCallIds?.has(message.toolCallId) ?? false; + const toolCallId = message.toolCallId; return (
= ({ result={message.result as BashToolResult | undefined} status={message.status} startedAt={message.timestamp} + canSendToBackground={canSendToBackground} + onSendToBackground={ + onSendBashToBackground ? () => onSendBashToBackground(toolCallId) : undefined + } />
); @@ -262,6 +283,18 @@ export const ToolMessage: React.FC = ({ ); } + if (isBashOutputTool(message.toolName, message.args)) { + return ( +
+ +
+ ); + } + // Fallback to generic tool call return (
diff --git a/src/browser/components/ReviewsBanner.tsx b/src/browser/components/ReviewsBanner.tsx index 7023cffc59..8e65953a3d 100644 --- a/src/browser/components/ReviewsBanner.tsx +++ b/src/browser/components/ReviewsBanner.tsx @@ -367,15 +367,17 @@ const ReviewsBannerInner: React.FC = ({ workspaceId }) diff --git a/src/browser/components/tools/BashBackgroundListToolCall.tsx b/src/browser/components/tools/BashBackgroundListToolCall.tsx index 7e9faefafb..896b644ad9 100644 --- a/src/browser/components/tools/BashBackgroundListToolCall.tsx +++ b/src/browser/components/tools/BashBackgroundListToolCall.tsx @@ -14,7 +14,6 @@ import { LoadingDots, ToolIcon, ErrorBox, - OutputPaths, } from "./shared/ToolPrimitives"; import { useToolExpansion, @@ -100,7 +99,6 @@ export const BashBackgroundListToolCall: React.FC {proc.script}
- ))} diff --git a/src/browser/components/tools/BashOutputToolCall.tsx b/src/browser/components/tools/BashOutputToolCall.tsx new file mode 100644 index 0000000000..de1ff9dbef --- /dev/null +++ b/src/browser/components/tools/BashOutputToolCall.tsx @@ -0,0 +1,90 @@ +import React from "react"; +import { Layers } from "lucide-react"; +import type { BashOutputToolArgs, BashOutputToolResult } from "@/common/types/tools"; +import { + ToolContainer, + ToolHeader, + ExpandIcon, + StatusIndicator, + ToolDetails, + DetailSection, + DetailLabel, + DetailContent, + LoadingDots, + ToolIcon, + ErrorBox, + OutputStatusBadge, + ProcessStatusBadge, + OutputSection, +} from "./shared/ToolPrimitives"; +import { useToolExpansion, getStatusDisplay, type ToolStatus } from "./shared/toolUtils"; + +interface BashOutputToolCallProps { + args: BashOutputToolArgs; + result?: BashOutputToolResult; + status?: ToolStatus; +} + +/** + * Display component for bash_output tool calls. + * Shows output from background processes in a format matching regular bash tool. + */ +export const BashOutputToolCall: React.FC = ({ + args, + result, + status = "pending", +}) => { + const { expanded, toggleExpanded } = useToolExpansion(); + + // Derive process status display + const processStatus = result?.success ? result.status : undefined; + + return ( + + + β–Ά + + {args.process_id} + + + output + {args.timeout_secs > 0 && ` β€’ wait ${args.timeout_secs}s`} + {args.filter && ` β€’ filter: ${args.filter}`} + + {result?.success && } + {result?.success && processStatus && processStatus !== "running" && ( + + )} + {getStatusDisplay(status)} + + + {expanded && ( + + {result && ( + <> + {result.success === false && ( + + Error + {result.error} + + )} + + {result.success && ( + + )} + + )} + + {status === "executing" && !result && ( + + + Waiting for result + + + + )} + + )} + + ); +}; diff --git a/src/browser/components/tools/BashToolCall.tsx b/src/browser/components/tools/BashToolCall.tsx index c23d6bdd8f..927d0d9851 100644 --- a/src/browser/components/tools/BashToolCall.tsx +++ b/src/browser/components/tools/BashToolCall.tsx @@ -1,4 +1,5 @@ import React, { useState, useEffect, useRef } from "react"; +import { Layers } from "lucide-react"; import type { BashToolArgs, BashToolResult } from "@/common/types/tools"; import { BASH_DEFAULT_TIMEOUT_SECS } from "@/common/constants/toolLimits"; import { @@ -13,7 +14,7 @@ import { LoadingDots, ToolIcon, ErrorBox, - OutputPaths, + ExitCodeBadge, } from "./shared/ToolPrimitives"; import { useToolExpansion, @@ -22,12 +23,17 @@ import { type ToolStatus, } from "./shared/toolUtils"; import { cn } from "@/common/lib/utils"; +import { Tooltip, TooltipTrigger, TooltipContent } from "../ui/tooltip"; interface BashToolCallProps { args: BashToolArgs; result?: BashToolResult; status?: ToolStatus; startedAt?: number; + /** Whether there's a foreground bash that can be sent to background */ + canSendToBackground?: boolean; + /** Callback to send the current foreground bash to background */ + onSendToBackground?: () => void; } export const BashToolCall: React.FC = ({ @@ -35,6 +41,8 @@ export const BashToolCall: React.FC = ({ result, status = "pending", startedAt, + canSendToBackground, + onSendToBackground, }) => { const { expanded, toggleExpanded } = useToolExpansion(); const [elapsedTime, setElapsedTime] = useState(0); @@ -61,18 +69,25 @@ export const BashToolCall: React.FC = ({ const isPending = status === "executing" || status === "pending"; const isBackground = args.run_in_background ?? (result && "backgroundProcessId" in result); + // Override status for backgrounded processes: the aggregator sees success=true and marks "completed", + // but for a foregroundβ†’background migration we want to show "backgrounded" + const effectiveStatus: ToolStatus = + status === "completed" && result && "backgroundProcessId" in result ? "backgrounded" : status; + return ( β–Ά {args.script} - {isBackground ? ( - // Background mode: show background badge and optional display name - - ⚑ background{args.display_name && ` β€’ ${args.display_name}`} + {isBackground && ( + // Background mode: show icon and display name + + + {args.display_name} - ) : ( + )} + {!isBackground && ( // Normal mode: show timeout and duration <> = ({ {result && ` β€’ took ${formatDuration(result.wall_duration_ms)}`} {!result && isPending && elapsedTime > 0 && ` β€’ ${Math.round(elapsedTime / 1000)}s`} - {result && ( - } + + )} + + {getStatusDisplay(effectiveStatus)} + + {/* Show "Background" button when bash is executing and can be sent to background. + Use invisible when executing but not yet confirmed as foreground to avoid layout flash. */} + {status === "executing" && !isBackground && onSendToBackground && ( + + + + + + Send to background β€” process continues but agent stops waiting + + )} - {getStatusDisplay(status)} {expanded && ( @@ -117,11 +152,14 @@ export const BashToolCall: React.FC = ({ )} {"backgroundProcessId" in result ? ( - // Background process: show file paths - - Output Files - - + // Background process: show process ID inline with icon (compact, no section wrapper) +
+ + Background process + + {result.backgroundProcessId} + +
) : ( // Normal process: show output result.output && ( diff --git a/src/browser/components/tools/shared/ToolPrimitives.tsx b/src/browser/components/tools/shared/ToolPrimitives.tsx index 13d112acc8..bd06699d14 100644 --- a/src/browser/components/tools/shared/ToolPrimitives.tsx +++ b/src/browser/components/tools/shared/ToolPrimitives.tsx @@ -70,6 +70,8 @@ const getStatusColor = (status: string) => { return "text-danger"; case "interrupted": return "text-interrupted"; + case "backgrounded": + return "text-backgrounded"; default: return "text-foreground-secondary"; } @@ -193,34 +195,97 @@ export const ErrorBox: React.FC> = ({ ); /** - * Output file paths display (stdout/stderr) - * @param compact - Use smaller text without background (for inline use in cards) + * Badge for displaying exit codes or process status */ -interface OutputPathsProps { - stdout: string; - stderr: string; - compact?: boolean; +interface ExitCodeBadgeProps { + exitCode: number; + className?: string; } -export const OutputPaths: React.FC = ({ stdout, stderr, compact }) => - compact ? ( -
-
- stdout: {stdout} -
-
- stderr: {stderr} -
-
- ) : ( -
-
- stdout:{" "} - {stdout} -
-
- stderr:{" "} - {stderr} -
-
+export const ExitCodeBadge: React.FC = ({ exitCode, className }) => ( + + {exitCode} + +); + +/** + * Badge for displaying process status (exited, killed, failed) + */ +interface ProcessStatusBadgeProps { + status: "exited" | "killed" | "failed"; + exitCode?: number; + className?: string; +} + +export const ProcessStatusBadge: React.FC = ({ + status, + exitCode, + className, +}) => ( + + {status} + {exitCode !== undefined && ` (${exitCode})`} + +); + +/** + * Badge for output availability status + */ +interface OutputStatusBadgeProps { + hasOutput: boolean; + className?: string; +} + +export const OutputStatusBadge: React.FC = ({ hasOutput, className }) => ( + + {hasOutput ? "new output" : "no output"} + +); + +/** + * Output display section for bash-like tools + */ +interface OutputSectionProps { + output?: string; + emptyMessage?: string; +} + +export const OutputSection: React.FC = ({ + output, + emptyMessage = "No output", +}) => { + if (output) { + return ( + + Output + {output} + + ); + } + + return ( + + {emptyMessage} + ); +}; diff --git a/src/browser/components/tools/shared/toolUtils.tsx b/src/browser/components/tools/shared/toolUtils.tsx index 1bcf262ea2..f5164f1e20 100644 --- a/src/browser/components/tools/shared/toolUtils.tsx +++ b/src/browser/components/tools/shared/toolUtils.tsx @@ -5,7 +5,13 @@ import { LoadingDots } from "./ToolPrimitives"; * Shared utilities and hooks for tool components */ -export type ToolStatus = "pending" | "executing" | "completed" | "failed" | "interrupted"; +export type ToolStatus = + | "pending" + | "executing" + | "completed" + | "failed" + | "interrupted" + | "backgrounded"; /** * Hook for managing tool expansion state @@ -45,6 +51,12 @@ export function getStatusDisplay(status: ToolStatus): React.ReactNode { ⚠ interrupted ); + case "backgrounded": + return ( + <> + β—Ž backgrounded + + ); default: return pending; } diff --git a/src/browser/hooks/useBackgroundBashHandlers.ts b/src/browser/hooks/useBackgroundBashHandlers.ts new file mode 100644 index 0000000000..6c536bca7d --- /dev/null +++ b/src/browser/hooks/useBackgroundBashHandlers.ts @@ -0,0 +1,168 @@ +import { useState, useEffect, useCallback, useMemo, useRef } from "react"; +import type { BackgroundProcessInfo } from "@/common/orpc/schemas/api"; +import type { APIClient } from "@/browser/contexts/API"; +import { usePopoverError } from "@/browser/hooks/usePopoverError"; + +/** Shared empty arrays/sets to avoid creating new objects */ +const EMPTY_SET = new Set(); +const EMPTY_PROCESSES: BackgroundProcessInfo[] = []; + +/** + * Hook to manage background bash processes and foreground-to-background transitions. + * + * Extracted from AIView to keep component size manageable. Encapsulates: + * - Subscribing to background process state changes (event-driven, no polling) + * - Terminating background processes + * - Detecting foreground bashes (by toolCallId) - supports multiple parallel processes + * - Sending foreground bash to background + * - Auto-backgrounding when new messages are sent + */ +export function useBackgroundBashHandlers( + api: APIClient | null, + workspaceId: string | null +): { + /** List of background processes */ + processes: BackgroundProcessInfo[]; + /** Terminate a background process */ + handleTerminate: (processId: string) => void; + /** Set of tool call IDs of foreground bashes */ + foregroundToolCallIds: Set; + /** Send a specific foreground bash to background */ + handleSendToBackground: (toolCallId: string) => void; + /** Handler to call when a message is sent (auto-backgrounds all foreground bashes) */ + handleMessageSentBackground: () => void; + /** Error state for popover display */ + error: ReturnType; +} { + const [processes, setProcesses] = useState(EMPTY_PROCESSES); + const [foregroundToolCallIds, setForegroundToolCallIds] = useState>(EMPTY_SET); + // Keep a ref for handleMessageSentBackground to avoid recreating on every change + const foregroundIdsRef = useRef>(EMPTY_SET); + const error = usePopoverError(); + + // Update ref when state changes (in effect to avoid running during render) + useEffect(() => { + foregroundIdsRef.current = foregroundToolCallIds; + }, [foregroundToolCallIds]); + + const terminate = useCallback( + async (processId: string): Promise => { + if (!api || !workspaceId) { + throw new Error("API or workspace not available"); + } + + const result = await api.workspace.backgroundBashes.terminate({ + workspaceId, + processId, + }); + if (!result.success) { + throw new Error(result.error); + } + // State will update via subscription + }, + [api, workspaceId] + ); + + const sendToBackground = useCallback( + async (toolCallId: string): Promise => { + if (!api || !workspaceId) { + throw new Error("API or workspace not available"); + } + + const result = await api.workspace.backgroundBashes.sendToBackground({ + workspaceId, + toolCallId, + }); + if (!result.success) { + throw new Error(result.error); + } + // State will update via subscription + }, + [api, workspaceId] + ); + + // Subscribe to background bash state changes + useEffect(() => { + if (!api || !workspaceId) { + setProcesses(EMPTY_PROCESSES); + setForegroundToolCallIds(EMPTY_SET); + return; + } + + const controller = new AbortController(); + const { signal } = controller; + + (async () => { + try { + const iterator = await api.workspace.backgroundBashes.subscribe( + { workspaceId }, + { signal } + ); + + for await (const state of iterator) { + if (signal.aborted) break; + + setProcesses(state.processes); + setForegroundToolCallIds(new Set(state.foregroundToolCallIds)); + } + } catch (err) { + if (!signal.aborted) { + console.error("Failed to subscribe to background bash state:", err); + } + } + })(); + + return () => { + controller.abort(); + }; + }, [api, workspaceId]); + + // Wrapped handlers with error handling + // Use error.showError directly in deps to avoid recreating when error.error changes + const { showError } = error; + const handleTerminate = useCallback( + (processId: string) => { + terminate(processId).catch((err: Error) => { + showError(processId, err.message); + }); + }, + [terminate, showError] + ); + + const handleSendToBackground = useCallback( + (toolCallId: string) => { + sendToBackground(toolCallId).catch((err: Error) => { + showError(`send-to-background-${toolCallId}`, err.message); + }); + }, + [sendToBackground, showError] + ); + + // Handler for when a message is sent - auto-background all foreground bashes + const handleMessageSentBackground = useCallback(() => { + for (const toolCallId of foregroundIdsRef.current) { + sendToBackground(toolCallId).catch(() => { + // Ignore errors - the bash might have finished just before we tried to background it + }); + } + }, [sendToBackground]); + + return useMemo( + () => ({ + processes, + handleTerminate, + foregroundToolCallIds, + handleSendToBackground, + handleMessageSentBackground, + error, + }), + [ + processes, + handleTerminate, + foregroundToolCallIds, + handleSendToBackground, + handleMessageSentBackground, + error, + ] + ); +} diff --git a/src/browser/hooks/usePopoverError.ts b/src/browser/hooks/usePopoverError.ts index f677942ea3..0d40c8efee 100644 --- a/src/browser/hooks/usePopoverError.ts +++ b/src/browser/hooks/usePopoverError.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; export interface PopoverErrorState { id: string; @@ -75,5 +75,5 @@ export function usePopoverError(autoDismissMs = 5000): UsePopoverErrorResult { }; }, [error, clearError]); - return { error, showError, clearError }; + return useMemo(() => ({ error, showError, clearError }), [error, showError, clearError]); } diff --git a/src/browser/hooks/useStableReference.ts b/src/browser/hooks/useStableReference.ts index 2bcbefbc9d..71fb6915a8 100644 --- a/src/browser/hooks/useStableReference.ts +++ b/src/browser/hooks/useStableReference.ts @@ -51,6 +51,21 @@ export function compareRecords( return true; } +/** + * Compare two Sets for equality (same size and values). + * + * @param prev Previous Set + * @param next Next Set + * @returns true if Sets are equal, false otherwise + */ +export function compareSets(a: Set, b: Set): boolean { + if (a.size !== b.size) return false; + for (const item of a) { + if (!b.has(item)) return false; + } + return true; +} + /** * Compare two Arrays for deep equality (same length and values). * Uses === for value comparison by default. diff --git a/src/browser/stories/App.bash.stories.tsx b/src/browser/stories/App.bash.stories.tsx new file mode 100644 index 0000000000..deb05ccf50 --- /dev/null +++ b/src/browser/stories/App.bash.stories.tsx @@ -0,0 +1,350 @@ +/** + * Bash tool stories - consolidated to 3 stories covering full UI complexity + */ + +import { appMeta, AppWithMocks, type AppStory } from "./meta.js"; +import { + STABLE_TIMESTAMP, + createUserMessage, + createAssistantMessage, + createBashTool, + createPendingTool, + createBackgroundBashTool, + createMigratedBashTool, + createBashOutputTool, + createBashOutputErrorTool, + createBashBackgroundListTool, + createBashBackgroundTerminateTool, +} from "./mockFactory"; +import { setupSimpleChatStory } from "./storyHelpers"; +import { userEvent, waitFor } from "@storybook/test"; + +/** + * Helper to expand all bash tool calls in a story. + * Clicks on the β–Ά expand icons to expand tool details. + */ +async function expandAllBashTools(canvasElement: HTMLElement) { + await waitFor( + async () => { + // Find all β–Ά expand icons (they contain the triangle character) + // The icon parent div is clickable and triggers expansion + const allSpans = canvasElement.querySelectorAll("span"); + const expandIcons = Array.from(allSpans).filter((span) => span.textContent?.trim() === "β–Ά"); + if (expandIcons.length === 0) { + throw new Error("No expand icons found"); + } + for (const icon of expandIcons) { + // Click the parent element (the tool header row) + const header = icon.closest("[class*='cursor-pointer']"); + if (header) { + await userEvent.click(header as HTMLElement); + } + } + }, + { timeout: 5000 } + ); + + // Wait for any auto-focus timers, then blur + await new Promise((resolve) => setTimeout(resolve, 150)); + (document.activeElement as HTMLElement)?.blur(); +} + +export default { + ...appMeta, + title: "App/Bash", +}; + +/** + * Foreground bash: complete execution with multi-line script + waiting state + */ +export const Foreground: AppStory = { + render: () => ( + + setupSimpleChatStory({ + workspaceId: "ws-bash", + messages: [ + // Completed foreground bash with multi-line script + createUserMessage("msg-1", "Check project status", { + historySequence: 1, + timestamp: STABLE_TIMESTAMP - 200000, + }), + createAssistantMessage("msg-2", "Let me check the git status and run tests:", { + historySequence: 2, + timestamp: STABLE_TIMESTAMP - 190000, + toolCalls: [ + createBashTool( + "call-1", + `#!/bin/bash +set -e + +# Check git status +echo "=== Git Status ===" +git status --short + +# Run tests +echo "=== Running Tests ===" +npm test 2>&1 | head -20`, + [ + "=== Git Status ===", + " M src/api/users.ts", + " M src/auth/jwt.ts", + "?? src/api/users.test.ts", + "", + "=== Running Tests ===", + "PASS src/api/users.test.ts", + " βœ“ should authenticate (24ms)", + " βœ“ should reject invalid tokens (18ms)", + "", + "Tests: 2 passed, 2 total", + ].join("\n"), + 0, + 10, + 1250 + ), + ], + }), + // Pending foreground bash (waiting state) + createUserMessage("msg-3", "Run the build", { + historySequence: 3, + timestamp: STABLE_TIMESTAMP - 100000, + }), + createAssistantMessage("msg-4", "Running the build:", { + historySequence: 4, + timestamp: STABLE_TIMESTAMP - 90000, + toolCalls: [ + createPendingTool("call-2", "bash", { + script: "npm run build", + run_in_background: false, + display_name: "Build", + timeout_secs: 60, + }), + ], + }), + ], + }) + } + /> + ), + play: async ({ canvasElement }: { canvasElement: HTMLElement }) => { + await expandAllBashTools(canvasElement); + }, +}; + +/** + * Background bash workflow: spawn, output states (running/exited/error/filtered/empty), + * process list, and terminate + */ +export const BackgroundWorkflow: AppStory = { + render: () => ( + + setupSimpleChatStory({ + messages: [ + // 1. Spawn background process + createUserMessage("msg-1", "Start a dev server and run build", { + historySequence: 1, + timestamp: STABLE_TIMESTAMP - 600000, + }), + createAssistantMessage("msg-2", "Starting both in background.", { + historySequence: 2, + timestamp: STABLE_TIMESTAMP - 590000, + toolCalls: [ + createBackgroundBashTool("call-1", "npm run dev", "bash_1", "Dev Server"), + createBackgroundBashTool("call-2", "npm run build", "bash_2", "Build"), + ], + }), + // 2. Output: running process + createUserMessage("msg-3", "Check dev server", { + historySequence: 3, + timestamp: STABLE_TIMESTAMP - 500000, + }), + createAssistantMessage("msg-4", "Dev server output:", { + historySequence: 4, + timestamp: STABLE_TIMESTAMP - 490000, + toolCalls: [ + createBashOutputTool( + "call-3", + "bash_1", + " VITE v5.0.0 ready in 320 ms\n\n ➜ Local: http://localhost:5173/\n ➜ Network: use --host to expose", + "running" + ), + ], + }), + // 3. Output: exited successfully + createUserMessage("msg-5", "Check build", { + historySequence: 5, + timestamp: STABLE_TIMESTAMP - 400000, + }), + createAssistantMessage("msg-6", "Build completed:", { + historySequence: 6, + timestamp: STABLE_TIMESTAMP - 390000, + toolCalls: [ + createBashOutputTool( + "call-4", + "bash_2", + "vite v5.0.0 building for production...\nβœ“ 1423 modules transformed.\ndist/index.html 0.46 kB β”‚ gzip: 0.30 kB\nβœ“ built in 2.34s", + "exited", + 0 + ), + ], + }), + // 4. Output: filtered + no new output + createUserMessage("msg-7", "Show errors from dev server, then check for updates", { + historySequence: 7, + timestamp: STABLE_TIMESTAMP - 300000, + }), + createAssistantMessage("msg-8", "Filtered errors and checking for updates:", { + historySequence: 8, + timestamp: STABLE_TIMESTAMP - 290000, + toolCalls: [ + createBashOutputTool( + "call-5", + "bash_1", + "[ERROR] Failed to connect to database\n[ERROR] Retry attempt 1 failed", + "running", + undefined, + "ERROR" + ), + createBashOutputTool("call-6", "bash_1", "", "running"), + ], + }), + // 5. Output: process not found error + createUserMessage("msg-9", "Check bash_99", { + historySequence: 9, + timestamp: STABLE_TIMESTAMP - 200000, + }), + createAssistantMessage("msg-10", "Checking that process:", { + historySequence: 10, + timestamp: STABLE_TIMESTAMP - 190000, + toolCalls: [ + createBashOutputErrorTool("call-7", "bash_99", "Process not found: bash_99"), + ], + }), + // 6. List all processes (shows various states) + createUserMessage("msg-11", "List all processes", { + historySequence: 11, + timestamp: STABLE_TIMESTAMP - 100000, + }), + createAssistantMessage("msg-12", "Background processes:", { + historySequence: 12, + timestamp: STABLE_TIMESTAMP - 90000, + toolCalls: [ + createBashBackgroundListTool("call-8", [ + { + process_id: "bash_1", + status: "running", + script: "npm run dev", + uptime_ms: 500000, + display_name: "Dev Server", + }, + { + process_id: "bash_2", + status: "exited", + script: "npm run build", + uptime_ms: 120000, + exitCode: 0, + }, + { + process_id: "bash_3", + status: "killed", + script: "npm run long-task", + uptime_ms: 45000, + exitCode: 143, + }, + ]), + ], + }), + // 7. Terminate + createUserMessage("msg-13", "Stop the dev server", { + historySequence: 13, + timestamp: STABLE_TIMESTAMP - 50000, + }), + createAssistantMessage("msg-14", "Terminating:", { + historySequence: 14, + timestamp: STABLE_TIMESTAMP - 40000, + toolCalls: [createBashBackgroundTerminateTool("call-9", "bash_1", "Dev Server")], + }), + ], + }) + } + /> + ), + play: async ({ canvasElement }: { canvasElement: HTMLElement }) => { + await expandAllBashTools(canvasElement); + }, +}; + +/** + * Mixed: foreground and background bash side-by-side comparison + */ +export const Mixed: AppStory = { + render: () => ( + + setupSimpleChatStory({ + messages: [ + createUserMessage("msg-1", "Run a quick command and a long one", { + historySequence: 1, + timestamp: STABLE_TIMESTAMP - 300000, + }), + createAssistantMessage( + "msg-2", + "I'll run the quick one normally and the long one in background.", + { + historySequence: 2, + timestamp: STABLE_TIMESTAMP - 290000, + toolCalls: [ + // Foreground: quick command + createBashTool("call-1", "echo 'Hello World'", "Hello World", 0, 3, 12), + // Background: long-running (explicit run_in_background=true) + createBackgroundBashTool("call-2", "npm run build && npm run test", "bash_6"), + ], + } + ), + // Migrated foregroundβ†’background (user clicked "Background" button) + createUserMessage("msg-3", "Run a long test suite", { + historySequence: 3, + timestamp: STABLE_TIMESTAMP - 200000, + }), + createAssistantMessage("msg-4", "Running tests:", { + historySequence: 4, + timestamp: STABLE_TIMESTAMP - 190000, + toolCalls: [ + // Shows "backgrounded" status (cyan) because it started as foreground + createMigratedBashTool( + "call-3", + "npm run test:integration", + "test-suite", + "Integration Tests", + "Running integration tests...\nTest 1: PASS\nTest 2: PASS\nTest 3: Running..." + ), + ], + }), + // Check background output + createUserMessage("msg-5", "How did the build go?", { + historySequence: 5, + timestamp: STABLE_TIMESTAMP - 100000, + }), + createAssistantMessage("msg-6", "The build failed:", { + historySequence: 6, + timestamp: STABLE_TIMESTAMP - 90000, + toolCalls: [ + createBashOutputTool( + "call-4", + "bash_6", + "FAIL src/utils.test.ts\n βœ• should parse dates correctly (5 ms)\n\nTests: 1 failed, 1 total", + "exited", + 1 + ), + ], + }), + ], + }) + } + /> + ), + play: async ({ canvasElement }: { canvasElement: HTMLElement }) => { + await expandAllBashTools(canvasElement); + }, +}; diff --git a/src/browser/stories/App.chat.stories.tsx b/src/browser/stories/App.chat.stories.tsx index c17c056f4a..32e4e39e53 100644 --- a/src/browser/stories/App.chat.stories.tsx +++ b/src/browser/stories/App.chat.stories.tsx @@ -10,11 +10,9 @@ import { createCompactionRequestMessage, createFileReadTool, createFileEditTool, - createBashTool, createTerminalTool, createStatusTool, createGenericTool, - createPendingTool, } from "./mockFactory"; import { setupSimpleChatStory, setupStreamingChatStory } from "./storyHelpers"; import { within, userEvent, waitFor } from "@storybook/test"; @@ -182,130 +180,6 @@ export const WithTerminal: AppStory = { ), }; -/** Bash tool with expanded script and output sections */ -export const WithBashTool: AppStory = { - render: () => ( - - setupSimpleChatStory({ - workspaceId: "ws-bash", - messages: [ - createUserMessage("msg-1", "Check project status", { - historySequence: 1, - timestamp: STABLE_TIMESTAMP - 100000, - }), - createAssistantMessage("msg-2", "Let me check the git status and run tests:", { - historySequence: 2, - timestamp: STABLE_TIMESTAMP - 90000, - toolCalls: [ - createBashTool( - "call-1", - `#!/bin/bash -set -e - -# Check git status -echo "=== Git Status ===" -git status --short - -# Run tests -echo "=== Running Tests ===" -npm test 2>&1 | head -20`, - [ - "=== Git Status ===", - " M src/api/users.ts", - " M src/auth/jwt.ts", - "?? src/api/users.test.ts", - "", - "=== Running Tests ===", - "PASS src/api/users.test.ts", - " βœ“ should authenticate (24ms)", - " βœ“ should reject invalid tokens (18ms)", - "", - "Tests: 2 passed, 2 total", - ].join("\n"), - 0, - 10, - 1250 - ), - ], - }), - ], - }) - } - /> - ), - parameters: { - docs: { - description: { - story: "Bash tool showing multi-line script in expanded view with proper padding.", - }, - }, - }, - play: async ({ canvasElement }: { canvasElement: HTMLElement }) => { - const canvas = within(canvasElement); - - // Expand the bash tool to show Script section with padding - await waitFor(async () => { - const toolHeader = canvas.getByText(/set -e/); - await userEvent.click(toolHeader); - }); - // Wait for any auto-focus timers (ChatInput has 100ms delay), then blur - await new Promise((resolve) => setTimeout(resolve, 150)); - (document.activeElement as HTMLElement)?.blur(); - }, -}; - -/** Bash tool in executing state showing "Waiting for result" */ -export const WithBashToolWaiting: AppStory = { - render: () => ( - - setupSimpleChatStory({ - workspaceId: "ws-bash-waiting", - messages: [ - createUserMessage("msg-1", "Run the tests", { - historySequence: 1, - timestamp: STABLE_TIMESTAMP - 100000, - }), - createAssistantMessage("msg-2", "Running the test suite:", { - historySequence: 2, - timestamp: STABLE_TIMESTAMP - 90000, - toolCalls: [ - createPendingTool("call-1", "bash", { - script: "npm test", - run_in_background: false, - display_name: "Test Runner", - timeout_secs: 30, - }), - ], - }), - ], - }) - } - /> - ), - parameters: { - docs: { - description: { - story: - "Bash tool in executing state with 'Waiting for result...' showing consistent padding.", - }, - }, - }, - play: async ({ canvasElement }: { canvasElement: HTMLElement }) => { - const canvas = within(canvasElement); - - // Expand the bash tool to show "Waiting for result" section - await waitFor(async () => { - const toolHeader = canvas.getByText(/npm test/); - await userEvent.click(toolHeader); - }); - // Wait for any auto-focus timers (ChatInput has 100ms delay), then blur - await new Promise((resolve) => setTimeout(resolve, 150)); - (document.activeElement as HTMLElement)?.blur(); - }, -}; - /** Chat with agent status indicator */ export const WithAgentStatus: AppStory = { render: () => ( @@ -446,10 +320,14 @@ export const GenericTool: AppStory = { play: async ({ canvasElement }: { canvasElement: HTMLElement }) => { const canvas = within(canvasElement); - await waitFor(async () => { - const toolHeader = canvas.getByText("fetch_data"); - await userEvent.click(toolHeader); - }); + // Wait for workspace metadata to load and main content to render + await waitFor( + async () => { + const toolHeader = canvas.getByText("fetch_data"); + await userEvent.click(toolHeader); + }, + { timeout: 5000 } + ); // Wait for any auto-focus timers (ChatInput has 100ms delay), then blur await new Promise((resolve) => setTimeout(resolve, 150)); (document.activeElement as HTMLElement)?.blur(); @@ -498,3 +376,70 @@ export const StreamingCompaction: AppStory = { }, }, }; + +/** Chat with running background processes banner */ +export const BackgroundProcesses: AppStory = { + render: () => ( + + setupSimpleChatStory({ + messages: [ + createUserMessage("msg-1", "Start the dev server and run tests in background", { + historySequence: 1, + timestamp: STABLE_TIMESTAMP - 60000, + }), + createAssistantMessage( + "msg-2", + "I've started the dev server and test runner in the background. You can continue working while they run.", + { + historySequence: 2, + timestamp: STABLE_TIMESTAMP - 50000, + toolCalls: [ + createTerminalTool( + "call-1", + "npm run dev &", + "Starting dev server on port 3000..." + ), + createTerminalTool("call-2", "npm test -- --watch &", "Running test suite..."), + ], + } + ), + ], + backgroundProcesses: [ + { + id: "bash_1", + pid: 12345, + script: "npm run dev", + displayName: "Dev Server", + startTime: Date.now() - 45000, // 45 seconds ago + status: "running", + }, + { + id: "bash_2", + pid: 12346, + script: "npm test -- --watch", + displayName: "Test Runner", + startTime: Date.now() - 30000, // 30 seconds ago + status: "running", + }, + { + id: "bash_3", + pid: 12347, + script: "tail -f /var/log/app.log", + startTime: Date.now() - 120000, // 2 minutes ago + status: "running", + }, + ], + }) + } + /> + ), + parameters: { + docs: { + description: { + story: + "Shows the background processes banner when there are running background bash processes. Click the banner to expand and see process details or terminate them.", + }, + }, + }, +}; diff --git a/src/browser/stories/mockFactory.ts b/src/browser/stories/mockFactory.ts index 3643623767..d3e1109370 100644 --- a/src/browser/stories/mockFactory.ts +++ b/src/browser/stories/mockFactory.ts @@ -333,6 +333,140 @@ export function createGenericTool( }; } +// ═══════════════════════════════════════════════════════════════════════════════ +// BACKGROUND BASH TOOL FACTORIES +// ═══════════════════════════════════════════════════════════════════════════════ + +/** Create a bash tool that spawns a background process */ +export function createBackgroundBashTool( + toolCallId: string, + script: string, + processId: string, + displayName?: string +): MuxPart { + return { + type: "dynamic-tool", + toolCallId, + toolName: "bash", + state: "output-available", + input: { script, run_in_background: true, display_name: displayName }, + output: { + success: true, + output: `Background process started with ID: ${processId}`, + exitCode: 0, + wall_duration_ms: 50, + backgroundProcessId: processId, + }, + }; +} + +/** Create a foreground bash that was migrated to background (user clicked "Background" button) */ +export function createMigratedBashTool( + toolCallId: string, + script: string, + processId: string, + displayName?: string, + capturedOutput?: string +): MuxPart { + const outputLines = capturedOutput?.split("\n") ?? []; + const outputSummary = + outputLines.length > 20 + ? `${outputLines.slice(-20).join("\n")}\n...(showing last 20 lines)` + : (capturedOutput ?? ""); + return { + type: "dynamic-tool", + toolCallId, + toolName: "bash", + state: "output-available", + // No run_in_background flag - this started as foreground + input: { script, run_in_background: false, display_name: displayName, timeout_secs: 30 }, + output: { + success: true, + output: `Process sent to background with ID: ${processId}\n\nOutput so far (${outputLines.length} lines):\n${outputSummary}`, + exitCode: 0, + wall_duration_ms: 5000, + backgroundProcessId: processId, // This triggers the "backgrounded" status + }, + }; +} + +/** Create a bash_output tool call showing process output */ +export function createBashOutputTool( + toolCallId: string, + processId: string, + output: string, + status: "running" | "exited" | "killed" | "failed" = "running", + exitCode?: number, + filter?: string +): MuxPart { + return { + type: "dynamic-tool", + toolCallId, + toolName: "bash_output", + state: "output-available", + input: { process_id: processId, filter }, + output: { success: true, status, output, exitCode }, + }; +} + +/** Create a bash_output tool call with error */ +export function createBashOutputErrorTool( + toolCallId: string, + processId: string, + error: string +): MuxPart { + return { + type: "dynamic-tool", + toolCallId, + toolName: "bash_output", + state: "output-available", + input: { process_id: processId }, + output: { success: false, error }, + }; +} + +/** Create a bash_background_list tool call */ +export function createBashBackgroundListTool( + toolCallId: string, + processes: Array<{ + process_id: string; + status: "running" | "exited" | "killed" | "failed"; + script: string; + uptime_ms: number; + exitCode?: number; + display_name?: string; + }> +): MuxPart { + return { + type: "dynamic-tool", + toolCallId, + toolName: "bash_background_list", + state: "output-available", + input: {}, + output: { success: true, processes }, + }; +} + +/** Create a bash_background_terminate tool call */ +export function createBashBackgroundTerminateTool( + toolCallId: string, + processId: string, + displayName?: string +): MuxPart { + return { + type: "dynamic-tool", + toolCallId, + toolName: "bash_background_terminate", + state: "output-available", + input: { process_id: processId }, + output: { + success: true, + message: `Process ${processId} terminated`, + display_name: displayName, + }, + }; +} + // ═══════════════════════════════════════════════════════════════════════════════ // GIT STATUS MOCKS // ═══════════════════════════════════════════════════════════════════════════════ diff --git a/src/browser/stories/storyHelpers.ts b/src/browser/stories/storyHelpers.ts index 0c0900765e..09cb81d50c 100644 --- a/src/browser/stories/storyHelpers.ts +++ b/src/browser/stories/storyHelpers.ts @@ -136,6 +136,16 @@ function createOnChatAdapter(chatHandlers: Map) { // SIMPLE CHAT STORY SETUP // ═══════════════════════════════════════════════════════════════════════════════ +export interface BackgroundProcessFixture { + id: string; + pid: number; + script: string; + displayName?: string; + startTime: number; + status: "running" | "exited" | "killed" | "failed"; + exitCode?: number; +} + export interface SimpleChatSetupOptions { workspaceId?: string; workspaceName?: string; @@ -143,6 +153,7 @@ export interface SimpleChatSetupOptions { messages: ChatMuxMessage[]; gitStatus?: GitStatusFixture; providersConfig?: Record; + backgroundProcesses?: BackgroundProcessFixture[]; } /** @@ -167,6 +178,11 @@ export function setupSimpleChatStory(opts: SimpleChatSetupOptions): APIClient { // Set localStorage for workspace selection selectWorkspace(workspaces[0]); + // Set up background processes map + const bgProcesses = opts.backgroundProcesses + ? new Map([[workspaceId, opts.backgroundProcesses]]) + : undefined; + // Return ORPC client return createMockORPCClient({ projects: groupWorkspacesByProject(workspaces), @@ -174,6 +190,7 @@ export function setupSimpleChatStory(opts: SimpleChatSetupOptions): APIClient { onChat: createOnChatAdapter(chatHandlers), executeBash: createGitStatusExecutor(gitStatus), providersConfig: opts.providersConfig, + backgroundProcesses: bgProcesses, }); } diff --git a/src/browser/styles/globals.css b/src/browser/styles/globals.css index 850f41e622..563ac8b5b6 100644 --- a/src/browser/styles/globals.css +++ b/src/browser/styles/globals.css @@ -158,6 +158,7 @@ /* Status */ --color-interrupted: hsl(38 92% 50%); + --color-backgrounded: hsl(200 70% 55%); /* cyan-ish blue for backgrounded processes */ --color-review-accent: hsl(48 70% 50%); --color-git-dirty: hsl(38 92% 50%); --color-error: hsl(0 70% 50%); @@ -394,6 +395,7 @@ --color-toggle-text-hover: hsl(210 20% 26%); --color-interrupted: hsl(38 92% 50%); + --color-backgrounded: hsl(200 70% 50%); --color-review-accent: hsl(48 70% 52%); --color-git-dirty: hsl(38 92% 50%); --color-error: hsl(0 68% 46%); @@ -622,6 +624,7 @@ --color-toggle-text-hover: #657b83; --color-interrupted: #cb4b16; /* orange */ + --color-backgrounded: #2aa198; /* cyan - solarized cyan */ --color-review-accent: #b58900; /* yellow */ --color-git-dirty: #cb4b16; --color-error: #dc322f; /* red */ @@ -842,6 +845,7 @@ --color-toggle-text-hover: #839496; --color-interrupted: #cb4b16; /* orange */ + --color-backgrounded: #2aa198; /* cyan - solarized cyan */ --color-review-accent: #b58900; /* yellow */ --color-git-dirty: #cb4b16; --color-error: #dc322f; /* red */ diff --git a/src/cli/run.ts b/src/cli/run.ts index 9b903a857d..f5ef06c943 100644 --- a/src/cli/run.ts +++ b/src/cli/run.ts @@ -9,6 +9,7 @@ */ import { Command } from "commander"; +import * as os from "os"; import * as path from "path"; import * as fs from "fs/promises"; import { Config } from "@/node/config"; @@ -297,7 +298,9 @@ async function main(): Promise { const historyService = new HistoryService(config); const partialService = new PartialService(config, historyService); const initStateManager = new InitStateManager(config); - const backgroundProcessManager = new BackgroundProcessManager(); + const backgroundProcessManager = new BackgroundProcessManager( + path.join(os.tmpdir(), "mux-bashes") + ); const aiService = new AIService( config, historyService, diff --git a/src/common/orpc/schemas/api.ts b/src/common/orpc/schemas/api.ts index 357ab9e04c..d7def1f7b6 100644 --- a/src/common/orpc/schemas/api.ts +++ b/src/common/orpc/schemas/api.ts @@ -28,6 +28,19 @@ export { telemetry, TelemetryEventSchema } from "./telemetry"; // --- API Router Schemas --- +// Background process info (for UI display) +export const BackgroundProcessInfoSchema = z.object({ + id: z.string(), + pid: z.number(), + script: z.string(), + displayName: z.string().optional(), + startTime: z.number(), + status: z.enum(["running", "exited", "killed", "failed"]), + exitCode: z.number().optional(), +}); + +export type BackgroundProcessInfo = z.infer; + // Tokenizer export const tokenizer = { countTokens: { @@ -308,6 +321,35 @@ export const workspace = { z.string() ), }, + backgroundBashes: { + /** + * Subscribe to background bash state changes for a workspace. + * Emits full state on connect, then incremental updates. + */ + subscribe: { + input: z.object({ workspaceId: z.string() }), + output: eventIterator( + z.object({ + /** Background processes (not including foreground ones being waited on) */ + processes: z.array(BackgroundProcessInfoSchema), + /** Tool call IDs of foreground bashes that can be sent to background */ + foregroundToolCallIds: z.array(z.string()), + }) + ), + }, + terminate: { + input: z.object({ workspaceId: z.string(), processId: z.string() }), + output: ResultSchema(z.void(), z.string()), + }, + /** + * Send a foreground bash process to background. + * The process continues running but the agent stops waiting for it. + */ + sendToBackground: { + input: z.object({ workspaceId: z.string(), toolCallId: z.string() }), + output: ResultSchema(z.void(), z.string()), + }, + }, }; export type WorkspaceSendMessageOutput = z.infer; diff --git a/src/common/types/tools.ts b/src/common/types/tools.ts index 06c44792fe..51ee98e2e9 100644 --- a/src/common/types/tools.ts +++ b/src/common/types/tools.ts @@ -11,7 +11,7 @@ export interface BashToolArgs { script: string; timeout_secs?: number; // Optional: defaults to 3 seconds for interactivity run_in_background?: boolean; // Run without blocking (for long-running processes) - display_name?: string; // Human-readable name for background processes + display_name: string; // Required - used as process identifier if sent to background } interface CommonBashFields { @@ -36,8 +36,6 @@ export type BashToolResult = output: string; exitCode: 0; backgroundProcessId: string; // Background spawn succeeded - stdout_path: string; // Path to stdout log file - stderr_path: string; // Path to stderr log file }) | (CommonBashFields & { success: false; @@ -227,6 +225,23 @@ export interface StatusSetToolArgs { url?: string; } +// Bash Output Tool Types (read incremental output from background processes) +export interface BashOutputToolArgs { + process_id: string; + filter?: string; + timeout_secs: number; +} + +export type BashOutputToolResult = + | { + success: true; + status: "running" | "exited" | "killed" | "failed"; + output: string; + exitCode?: number; + note?: string; // Agent-only message (not displayed in UI) + } + | { success: false; error: string }; + // Bash Background Tool Types export interface BashBackgroundTerminateArgs { process_id: string; @@ -245,8 +260,6 @@ export interface BashBackgroundListProcess { script: string; uptime_ms: number; exitCode?: number; - stdout_path: string; // Path to stdout log file - stderr_path: string; // Path to stderr log file display_name?: string; // Human-readable name (e.g., "Dev Server") } diff --git a/src/common/utils/asyncEventIterator.ts b/src/common/utils/asyncEventIterator.ts new file mode 100644 index 0000000000..5e2124988f --- /dev/null +++ b/src/common/utils/asyncEventIterator.ts @@ -0,0 +1,133 @@ +/** + * Convert event emitter subscription to async iterator. + * + * Handles the common pattern of: + * 1. Subscribe to events + * 2. Yield events as async iterator + * 3. Unsubscribe on cleanup + * + * Usage: + * ```ts + * yield* asyncEventIterator( + * (handler) => emitter.on('event', handler), + * (handler) => emitter.off('event', handler) + * ); + * ``` + * + * Or with initialValue for immediate first yield: + * ```ts + * yield* asyncEventIterator( + * (handler) => service.onChange(handler), + * (handler) => service.offChange(handler), + * { initialValue: await service.getState() } + * ); + * ``` + */ +export async function* asyncEventIterator( + subscribe: (handler: (value: T) => void) => void, + unsubscribe: (handler: (value: T) => void) => void, + options?: { initialValue?: T } +): AsyncGenerator { + const queue: T[] = []; + let resolveNext: ((value: T) => void) | null = null; + let ended = false; + + const handler = (value: T) => { + if (ended) return; + if (resolveNext) { + const resolve = resolveNext; + resolveNext = null; + resolve(value); + } else { + queue.push(value); + } + }; + + subscribe(handler); + + try { + // Yield initial value if provided + if (options?.initialValue !== undefined) { + yield options.initialValue; + } + + while (!ended) { + if (queue.length > 0) { + yield queue.shift()!; + } else { + yield await new Promise((resolve) => { + resolveNext = resolve; + }); + } + } + } finally { + ended = true; + unsubscribe(handler); + } +} + +/** + * Create an async event queue that can be pushed to from event handlers. + * + * This is useful when events don't directly yield values but trigger + * async state fetches. + * + * Usage: + * ```ts + * const queue = createAsyncEventQueue(); + * + * const onChange = async () => { + * queue.push(await fetchState()); + * }; + * + * emitter.on('change', onChange); + * try { + * yield* queue.iterate(); + * } finally { + * emitter.off('change', onChange); + * } + * ``` + */ +export function createAsyncEventQueue(): { + push: (value: T) => void; + iterate: () => AsyncGenerator; + end: () => void; +} { + const queue: T[] = []; + let resolveNext: ((value: T) => void) | null = null; + let ended = false; + + const push = (value: T) => { + if (ended) return; + if (resolveNext) { + const resolve = resolveNext; + resolveNext = null; + resolve(value); + } else { + queue.push(value); + } + }; + + async function* iterate(): AsyncGenerator { + while (!ended) { + if (queue.length > 0) { + yield queue.shift()!; + } else { + yield await new Promise((resolve) => { + resolveNext = resolve; + }); + } + } + } + + const end = () => { + ended = true; + // Wake up the iterator if it's waiting + if (resolveNext) { + // This will never be yielded since ended=true stops the loop + resolveNext(undefined as T); + } + }; + + return { push, iterate, end }; +} diff --git a/src/common/utils/tools/toolDefinitions.ts b/src/common/utils/tools/toolDefinitions.ts index f37cead657..ba2f6762db 100644 --- a/src/common/utils/tools/toolDefinitions.ts +++ b/src/common/utils/tools/toolDefinitions.ts @@ -7,7 +7,6 @@ import { z } from "zod"; import { - BASH_DEFAULT_TIMEOUT_SECS, BASH_HARD_MAX_LINES, BASH_MAX_LINE_BYTES, BASH_MAX_TOTAL_BYTES, @@ -48,9 +47,10 @@ export const TOOL_DEFINITIONS = { timeout_secs: z .number() .positive() - .optional() .describe( - `Timeout (seconds, default: ${BASH_DEFAULT_TIMEOUT_SECS}). Start small and increase on retry; avoid large initial values to keep UX responsive` + "Timeout in seconds. For foreground: max execution time before kill. " + + "For background: max lifetime before auto-termination. " + + "Start small and increase on retry; avoid large initial values to keep UX responsive" ), run_in_background: z .boolean() @@ -60,17 +60,18 @@ export const TOOL_DEFINITIONS = { "Use for processes running >5s (dev servers, builds, file watchers). " + "Do NOT use for quick commands (<5s), interactive processes (no stdin support), " + "or processes requiring real-time output (use foreground with larger timeout instead). " + - "Returns immediately with process_id (e.g., bg-a1b2c3d4), stdout_path, and stderr_path. " + - "Read output with bash (e.g., tail -50 ). " + + "Returns immediately with process_id. " + + "Read output with bash_output (returns only new output since last check). " + "Terminate with bash_background_terminate using the process_id. " + - "Process persists until terminated or workspace is removed." + "Process persists until timeout_secs expires, terminated, or workspace is removed." + + "\\n\\nFor long-running tasks like builds or compilations, prefer background mode to continue productive work in parallel. " + + "Check back periodically with bash_output rather than blocking on completion." ), display_name: z .string() - .optional() .describe( - "Human-readable name for background processes (e.g., 'Dev Server', 'TypeCheck Watch'). " + - "Only used when run_in_background=true." + "Human-readable name for the process (e.g., 'Dev Server', 'TypeCheck Watch'). " + + "Required for all bash invocations since any process can be sent to background." ), }), }, @@ -236,11 +237,40 @@ export const TOOL_DEFINITIONS = { }) .strict(), }, + bash_output: { + description: + "Retrieve output from a running or completed background bash process. " + + "Returns only NEW output since the last check (incremental). " + + "Returns stdout and stderr output along with process status. " + + "Supports optional regex filtering to show only lines matching a pattern. " + + "WARNING: When using filter, non-matching lines are permanently discarded. " + + "Use timeout to wait for output instead of polling repeatedly.", + schema: z.object({ + process_id: z.string().describe("The ID of the background process to retrieve output from"), + filter: z + .string() + .optional() + .describe( + "Optional regex to filter output lines. Only matching lines are returned. " + + "Non-matching lines are permanently discarded and cannot be retrieved later." + ), + timeout_secs: z + .number() + .min(0) + .max(15) + .describe( + "Seconds to wait for new output (0-15). " + + "If no output is immediately available and process is still running, " + + "blocks up to this duration. Returns early when output arrives or process exits. " + + "Use this instead of polling in a loop." + ), + }), + }, bash_background_list: { description: "List all background processes started with bash(run_in_background=true). " + - "Returns process_id, status, script, stdout_path, stderr_path for each process. " + - "Use to find process_id for termination or check output file paths.", + "Returns process_id, status, script for each process. " + + "Use to find process_id for termination or check output with bash_output.", schema: z.object({}), }, bash_background_terminate: { @@ -248,12 +278,9 @@ export const TOOL_DEFINITIONS = { "Terminate a background process started with bash(run_in_background=true). " + "Use process_id from the original bash response or from bash_background_list. " + "Sends SIGTERM, waits briefly, then SIGKILL if needed. " + - "Output files remain available after termination.", + "Output remains available via bash_output after termination.", schema: z.object({ - process_id: z - .string() - .regex(/^bg-[0-9a-f]{8}$/, "Invalid process ID format") - .describe("Background process ID to terminate"), + process_id: z.string().describe("Background process ID to terminate"), }), }, web_fetch: { @@ -299,6 +326,7 @@ export function getAvailableTools(modelString: string): string[] { // Base tools available for all models const baseTools = [ "bash", + "bash_output", "bash_background_list", "bash_background_terminate", "file_read", diff --git a/src/common/utils/tools/tools.ts b/src/common/utils/tools/tools.ts index 0bdf428cf4..922ea06bfa 100644 --- a/src/common/utils/tools/tools.ts +++ b/src/common/utils/tools/tools.ts @@ -1,6 +1,7 @@ import { type Tool } from "ai"; import { createFileReadTool } from "@/node/services/tools/file_read"; import { createBashTool } from "@/node/services/tools/bash"; +import { createBashOutputTool } from "@/node/services/tools/bash_output"; import { createBashBackgroundListTool } from "@/node/services/tools/bash_background_list"; import { createBashBackgroundTerminateTool } from "@/node/services/tools/bash_background_terminate"; import { createFileEditReplaceStringTool } from "@/node/services/tools/file_edit_replace_string"; @@ -119,6 +120,7 @@ export async function getToolsForModel( // and line number miscalculations. Use file_edit_replace_string instead. // file_edit_replace_lines: wrap(createFileEditReplaceLinesTool(config)), bash: wrap(createBashTool(config)), + bash_output: wrap(createBashOutputTool(config)), bash_background_list: wrap(createBashBackgroundListTool(config)), bash_background_terminate: wrap(createBashBackgroundTerminateTool(config)), web_fetch: wrap(createWebFetchTool(config)), diff --git a/src/node/orpc/router.ts b/src/node/orpc/router.ts index a04c84d90b..2c232416be 100644 --- a/src/node/orpc/router.ts +++ b/src/node/orpc/router.ts @@ -16,6 +16,7 @@ import { createAuthMiddleware } from "./authMiddleware"; import { createAsyncMessageQueue } from "@/common/utils/asyncMessageQueue"; import { getPlanFilePath, readPlanFile } from "@/common/utils/planStorage"; import { findAvailableCommand, GUI_EDITORS, TERMINAL_EDITORS } from "@/node/utils/commandDiscovery"; +import { createAsyncEventQueue } from "@/common/utils/asyncEventIterator"; export const router = (authToken?: string) => { const t = os.$context().use(createAuthMiddleware(authToken)); @@ -691,6 +692,62 @@ export const router = (authToken?: string) => { } return { success: true as const, data: { content, path: planPath } }; }), + backgroundBashes: { + subscribe: t + .input(schemas.workspace.backgroundBashes.subscribe.input) + .output(schemas.workspace.backgroundBashes.subscribe.output) + .handler(async function* ({ context, input }) { + const service = context.workspaceService; + const { workspaceId } = input; + + const getState = async () => ({ + processes: await service.listBackgroundProcesses(workspaceId), + foregroundToolCallIds: service.getForegroundToolCallIds(workspaceId), + }); + + const queue = createAsyncEventQueue>>(); + + const onChange = (changedWorkspaceId: string) => { + if (changedWorkspaceId === workspaceId) { + void getState().then(queue.push); + } + }; + + service.onBackgroundBashChange(onChange); + + try { + // Emit initial state immediately + yield await getState(); + yield* queue.iterate(); + } finally { + queue.end(); + service.offBackgroundBashChange(onChange); + } + }), + terminate: t + .input(schemas.workspace.backgroundBashes.terminate.input) + .output(schemas.workspace.backgroundBashes.terminate.output) + .handler(async ({ context, input }) => { + const result = await context.workspaceService.terminateBackgroundProcess( + input.workspaceId, + input.processId + ); + if (!result.success) { + return { success: false, error: result.error }; + } + return { success: true, data: undefined }; + }), + sendToBackground: t + .input(schemas.workspace.backgroundBashes.sendToBackground.input) + .output(schemas.workspace.backgroundBashes.sendToBackground.output) + .handler(({ context, input }) => { + const result = context.workspaceService.sendToBackground(input.toolCallId); + if (!result.success) { + return { success: false, error: result.error }; + } + return { success: true, data: undefined }; + }), + }, }, window: { setTitle: t @@ -844,36 +901,13 @@ export const router = (authToken?: string) => { .input(schemas.update.onStatus.input) .output(schemas.update.onStatus.output) .handler(async function* ({ context }) { - let resolveNext: ((value: UpdateStatus) => void) | null = null; - const queue: UpdateStatus[] = []; - let ended = false; - - const push = (status: UpdateStatus) => { - if (ended) return; - if (resolveNext) { - const resolve = resolveNext; - resolveNext = null; - resolve(status); - } else { - queue.push(status); - } - }; - - const unsubscribe = context.updateService.onStatus(push); + const queue = createAsyncEventQueue(); + const unsubscribe = context.updateService.onStatus(queue.push); try { - while (!ended) { - if (queue.length > 0) { - yield queue.shift()!; - } else { - const status = await new Promise((resolve) => { - resolveNext = resolve; - }); - yield status; - } - } + yield* queue.iterate(); } finally { - ended = true; + queue.end(); unsubscribe(); } }), @@ -883,29 +917,16 @@ export const router = (authToken?: string) => { .input(schemas.menu.onOpenSettings.input) .output(schemas.menu.onOpenSettings.output) .handler(async function* ({ context }) { - let resolveNext: (() => void) | null = null; - let ended = false; - - const push = () => { - if (ended) return; - if (resolveNext) { - const resolve = resolveNext; - resolveNext = null; - resolve(); - } - }; - - const unsubscribe = context.menuEventService.onOpenSettings(push); + // Use a sentinel value to signal events since void/undefined can't be queued + const queue = createAsyncEventQueue(); + const unsubscribe = context.menuEventService.onOpenSettings(() => queue.push(true)); try { - while (!ended) { - await new Promise((resolve) => { - resolveNext = resolve; - }); + for await (const _ of queue.iterate()) { yield undefined; } } finally { - ended = true; + queue.end(); unsubscribe(); } }), diff --git a/src/node/runtime/LocalBackgroundHandle.ts b/src/node/runtime/LocalBackgroundHandle.ts deleted file mode 100644 index 41a8c221c8..0000000000 --- a/src/node/runtime/LocalBackgroundHandle.ts +++ /dev/null @@ -1,81 +0,0 @@ -import type { BackgroundHandle } from "./Runtime"; -import { parseExitCode, buildTerminateCommand } from "./backgroundCommands"; -import { log } from "@/node/services/log"; -import { execAsync } from "@/node/utils/disposableExec"; -import { getBashPath } from "@/node/utils/main/bashPath"; -import * as fs from "fs/promises"; -import * as path from "path"; - -/** - * Handle to a local background process. - * - * Uses file-based status detection (same approach as SSHBackgroundHandle): - * - Process is running if exit_code file doesn't exist - * - Exit code is read from exit_code file (written by bash trap on exit) - * - * Output is written directly to files via shell redirection (nohup ... > file), - * so the process continues writing even if mux closes. - */ -export class LocalBackgroundHandle implements BackgroundHandle { - private terminated = false; - - constructor( - private readonly pid: number, - public readonly outputDir: string - ) {} - - /** - * Get the exit code from the exit_code file. - * Returns null if process is still running (file doesn't exist yet). - */ - async getExitCode(): Promise { - try { - const exitCodePath = path.join(this.outputDir, "exit_code"); - const content = await fs.readFile(exitCodePath, "utf-8"); - return parseExitCode(content); - } catch { - // File doesn't exist or can't be read - process still running or crashed - return null; - } - } - - /** - * Terminate the process by killing the process group. - * Sends SIGTERM (15), waits 2 seconds, then SIGKILL (9) if still running. - * - * Uses buildTerminateCommand for parity with SSH - works on Linux, macOS, and Windows MSYS2. - */ - async terminate(): Promise { - if (this.terminated) return; - - try { - const exitCodePath = path.join(this.outputDir, "exit_code"); - const terminateCmd = buildTerminateCommand(this.pid, exitCodePath); - log.debug(`LocalBackgroundHandle: Terminating process group ${this.pid}`); - using proc = execAsync(terminateCmd, { shell: getBashPath() }); - await proc.result; - } catch (error) { - // Process may already be dead - that's fine - log.debug( - `LocalBackgroundHandle.terminate: Error: ${error instanceof Error ? error.message : String(error)}` - ); - } - - this.terminated = true; - } - - /** - * Clean up resources. - * No local resources to clean - process runs independently via nohup. - */ - async dispose(): Promise { - // No resources to clean up - we don't own the process - } - - /** - * Write meta.json to the output directory. - */ - async writeMeta(metaJson: string): Promise { - await fs.writeFile(path.join(this.outputDir, "meta.json"), metaJson); - } -} diff --git a/src/node/runtime/LocalBaseRuntime.ts b/src/node/runtime/LocalBaseRuntime.ts index 2116841e47..0dc047892b 100644 --- a/src/node/runtime/LocalBaseRuntime.ts +++ b/src/node/runtime/LocalBaseRuntime.ts @@ -3,7 +3,7 @@ import * as fs from "fs"; import * as fsPromises from "fs/promises"; import * as path from "path"; import { Readable, Writable } from "stream"; -import { randomBytes } from "crypto"; + import type { Runtime, ExecOptions, @@ -16,20 +16,14 @@ import type { WorkspaceForkParams, WorkspaceForkResult, InitLogger, - BackgroundSpawnOptions, - BackgroundSpawnResult, } from "./Runtime"; import { RuntimeError as RuntimeErrorClass } from "./Runtime"; import { NON_INTERACTIVE_ENV_VARS } from "@/common/constants/env"; import { getBashPath } from "@/node/utils/main/bashPath"; import { EXIT_CODE_ABORTED, EXIT_CODE_TIMEOUT } from "@/common/constants/exitCodes"; -import { DisposableProcess, execAsync } from "@/node/utils/disposableExec"; +import { DisposableProcess } from "@/node/utils/disposableExec"; import { expandTilde } from "./tildeExpansion"; import { getInitHookPath, createLineBufferedLoggers } from "./initHook"; -import { LocalBackgroundHandle } from "./LocalBackgroundHandle"; -import { buildWrapperScript, buildSpawnCommand, parsePid } from "./backgroundCommands"; -import { log } from "@/node/services/log"; -import { toPosixPath } from "@/node/utils/paths"; /** * Abstract base class for local runtimes (both WorktreeRuntime and LocalRuntime). @@ -51,12 +45,6 @@ import { toPosixPath } from "@/node/utils/paths"; * - forkWorkspace() */ export abstract class LocalBaseRuntime implements Runtime { - protected readonly bgOutputDir: string; - - constructor(bgOutputDir: string) { - this.bgOutputDir = expandTilde(bgOutputDir); - } - async exec(command: string, options: ExecOptions): Promise { const startTime = performance.now(); @@ -327,73 +315,6 @@ export abstract class LocalBaseRuntime implements Runtime { return path.resolve(basePath, target); } - /** - * Spawn a background process that persists independently of mux. - * Output is written to files in bgOutputDir/{workspaceId}/{processId}/. - */ - async spawnBackground( - script: string, - options: BackgroundSpawnOptions - ): Promise { - log.debug(`LocalBaseRuntime.spawnBackground: Spawning in ${options.cwd}`); - - // Check if working directory exists - try { - await fsPromises.access(options.cwd); - } catch { - return { success: false, error: `Working directory does not exist: ${options.cwd}` }; - } - - // Generate unique process ID and compute output directory - const processId = `bg-${randomBytes(4).toString("hex")}`; - const outputDir = path.join(this.bgOutputDir, options.workspaceId, processId); - const stdoutPath = path.join(outputDir, "stdout.log"); - const stderrPath = path.join(outputDir, "stderr.log"); - const exitCodePath = path.join(outputDir, "exit_code"); - - // Create output directory and empty files - await fsPromises.mkdir(outputDir, { recursive: true }); - await fsPromises.writeFile(stdoutPath, ""); - await fsPromises.writeFile(stderrPath, ""); - - // Build wrapper script and spawn command using shared builders (same as SSH for parity) - // On Windows, convert paths to POSIX format for Git Bash (C:\foo β†’ /c/foo) - const wrapperScript = buildWrapperScript({ - exitCodePath: toPosixPath(exitCodePath), - cwd: toPosixPath(options.cwd), - env: { ...options.env, ...NON_INTERACTIVE_ENV_VARS }, - script, - }); - - const spawnCommand = buildSpawnCommand({ - wrapperScript, - stdoutPath: toPosixPath(stdoutPath), - stderrPath: toPosixPath(stderrPath), - bashPath: getBashPath(), - niceness: options.niceness, - }); - - try { - // Use bash shell explicitly - spawnCommand uses POSIX commands (nohup, ps) - using proc = execAsync(spawnCommand, { shell: getBashPath() }); - const result = await proc.result; - - const pid = parsePid(result.stdout); - if (!pid) { - log.debug(`LocalBaseRuntime.spawnBackground: Invalid PID: ${result.stdout}`); - return { success: false, error: `Failed to get valid PID from spawn: ${result.stdout}` }; - } - - log.debug(`LocalBaseRuntime.spawnBackground: Spawned with PID ${pid}`); - const handle = new LocalBackgroundHandle(pid, outputDir); - return { success: true, handle, pid }; - } catch (e) { - const err = e as Error; - log.debug(`LocalBaseRuntime.spawnBackground: Failed to spawn: ${err.message}`); - return { success: false, error: err.message }; - } - } - // Abstract methods that subclasses must implement abstract getWorkspacePath(projectPath: string, workspaceName: string): string; @@ -419,6 +340,16 @@ export abstract class LocalBaseRuntime implements Runtime { abstract forkWorkspace(params: WorkspaceForkParams): Promise; + /** + * Get the runtime's temp directory. + * Uses OS temp dir on local systems. + */ + tempDir(): Promise { + // Use /tmp on Unix, or OS temp dir on Windows + const isWindows = process.platform === "win32"; + return Promise.resolve(isWindows ? (process.env.TEMP ?? "C:\\Temp") : "/tmp"); + } + /** * Helper to run .mux/init hook if it exists and is executable. * Shared between WorktreeRuntime and LocalRuntime. diff --git a/src/node/runtime/LocalRuntime.test.ts b/src/node/runtime/LocalRuntime.test.ts index b814a080c0..4aec268cdf 100644 --- a/src/node/runtime/LocalRuntime.test.ts +++ b/src/node/runtime/LocalRuntime.test.ts @@ -38,7 +38,7 @@ describe("LocalRuntime", () => { describe("constructor and getWorkspacePath", () => { it("stores projectPath and returns it regardless of arguments", () => { - const runtime = new LocalRuntime("/home/user/my-project", testDir); + const runtime = new LocalRuntime("/home/user/my-project"); // Both arguments are ignored - always returns the project path expect(runtime.getWorkspacePath("/other/path", "some-branch")).toBe("/home/user/my-project"); expect(runtime.getWorkspacePath("", "")).toBe("/home/user/my-project"); @@ -46,14 +46,14 @@ describe("LocalRuntime", () => { it("does not expand tilde (unlike WorktreeRuntime)", () => { // LocalRuntime stores the path as-is; callers must pass expanded paths - const runtime = new LocalRuntime("~/my-project", testDir); + const runtime = new LocalRuntime("~/my-project"); expect(runtime.getWorkspacePath("", "")).toBe("~/my-project"); }); }); describe("createWorkspace", () => { it("succeeds when directory exists", async () => { - const runtime = new LocalRuntime(testDir, testDir); + const runtime = new LocalRuntime(testDir); const logger = createMockLogger(); const result = await runtime.createWorkspace({ @@ -72,7 +72,7 @@ describe("LocalRuntime", () => { it("fails when directory does not exist", async () => { const nonExistentPath = path.join(testDir, "does-not-exist"); - const runtime = new LocalRuntime(nonExistentPath, testDir); + const runtime = new LocalRuntime(nonExistentPath); const logger = createMockLogger(); const result = await runtime.createWorkspace({ @@ -90,7 +90,7 @@ describe("LocalRuntime", () => { describe("deleteWorkspace", () => { it("returns success without deleting anything", async () => { - const runtime = new LocalRuntime(testDir, testDir); + const runtime = new LocalRuntime(testDir); // Create a test file to verify it isn't deleted const testFile = path.join(testDir, "delete-test.txt"); @@ -115,7 +115,7 @@ describe("LocalRuntime", () => { }); it("returns success even with force=true (still no-op)", async () => { - const runtime = new LocalRuntime(testDir, testDir); + const runtime = new LocalRuntime(testDir); const result = await runtime.deleteWorkspace(testDir, "main", true); @@ -134,7 +134,7 @@ describe("LocalRuntime", () => { describe("renameWorkspace", () => { it("is a no-op that returns success with same path", async () => { - const runtime = new LocalRuntime(testDir, testDir); + const runtime = new LocalRuntime(testDir); const result = await runtime.renameWorkspace(testDir, "old", "new"); @@ -148,7 +148,7 @@ describe("LocalRuntime", () => { describe("forkWorkspace", () => { it("returns error - operation not supported", async () => { - const runtime = new LocalRuntime(testDir, testDir); + const runtime = new LocalRuntime(testDir); const logger = createMockLogger(); const result = await runtime.forkWorkspace({ @@ -166,7 +166,7 @@ describe("LocalRuntime", () => { describe("inherited LocalBaseRuntime methods", () => { it("exec runs commands in projectPath", async () => { - const runtime = new LocalRuntime(testDir, testDir); + const runtime = new LocalRuntime(testDir); const stream = await runtime.exec("pwd", { cwd: testDir, @@ -187,7 +187,7 @@ describe("LocalRuntime", () => { }); it("stat works on projectPath", async () => { - const runtime = new LocalRuntime(testDir, testDir); + const runtime = new LocalRuntime(testDir); const stat = await runtime.stat(testDir); @@ -195,7 +195,7 @@ describe("LocalRuntime", () => { }); it("resolvePath expands tilde", async () => { - const runtime = new LocalRuntime(testDir, testDir); + const runtime = new LocalRuntime(testDir); const resolved = await runtime.resolvePath("~"); @@ -203,7 +203,7 @@ describe("LocalRuntime", () => { }); it("normalizePath resolves relative paths", () => { - const runtime = new LocalRuntime(testDir, testDir); + const runtime = new LocalRuntime(testDir); const result = runtime.normalizePath(".", testDir); diff --git a/src/node/runtime/LocalRuntime.ts b/src/node/runtime/LocalRuntime.ts index eb97c65f5f..aaabea879f 100644 --- a/src/node/runtime/LocalRuntime.ts +++ b/src/node/runtime/LocalRuntime.ts @@ -25,8 +25,8 @@ import { LocalBaseRuntime } from "./LocalBaseRuntime"; export class LocalRuntime extends LocalBaseRuntime { private readonly projectPath: string; - constructor(projectPath: string, bgOutputDir: string) { - super(bgOutputDir); + constructor(projectPath: string) { + super(); this.projectPath = projectPath; } diff --git a/src/node/runtime/Runtime.ts b/src/node/runtime/Runtime.ts index 39050d7995..71754f181f 100644 --- a/src/node/runtime/Runtime.ts +++ b/src/node/runtime/Runtime.ts @@ -67,29 +67,15 @@ export interface ExecOptions { forcePTY?: boolean; } -/** - * Options for spawning a background process - */ -export interface BackgroundSpawnOptions { - /** Working directory for command execution */ - cwd: string; - /** Workspace ID for output directory organization */ - workspaceId: string; - /** Environment variables to inject */ - env?: Record; - /** Process niceness level (-20 to 19, lower = higher priority) */ - niceness?: number; -} - /** * Handle to a background process. * Abstracts away whether process is local or remote. * - * Output is written directly to files by the runtime. + * Output is written directly to a unified output.log file by shell redirection. * This handle is for lifecycle management and output directory operations. */ export interface BackgroundHandle { - /** Output directory containing stdout.log, stderr.log, meta.json */ + /** Output directory containing output.log, meta.json, exit_code */ readonly outputDir: string; /** @@ -113,14 +99,14 @@ export interface BackgroundHandle { * Write meta.json to the output directory. */ writeMeta(metaJson: string): Promise; -} -/** - * Result of spawning a background process - */ -export type BackgroundSpawnResult = - | { success: true; handle: BackgroundHandle; pid: number } - | { success: false; error: string }; + /** + * Read output from output.log at the given byte offset. + * Returns the content read and the new offset (for incremental reads). + * Works on both local and SSH runtimes by using runtime.exec() internally. + */ + readOutput(offset: number): Promise<{ content: string; newOffset: number }>; +} /** * Streaming result from executing a command @@ -269,21 +255,6 @@ export interface Runtime { */ exec(command: string, options: ExecOptions): Promise; - /** - * Spawn a detached background process. - * Returns a handle for monitoring output and terminating the process. - * Unlike exec(), background processes have no timeout and run until terminated. - * - * Output directory is determined by runtime implementation: - * - LocalRuntime: {bgOutputDir}/{workspaceId}/{processId}/ (default: /tmp/mux-bashes) - * - SSHRuntime: {bgOutputDir}/{workspaceId}/{processId}/ (default: /tmp/mux-bashes) - * - * @param script Bash script to execute - * @param options Execution options (cwd, workspaceId, processId, env, niceness) - * @returns BackgroundHandle on success, or error - */ - spawnBackground(script: string, options: BackgroundSpawnOptions): Promise; - /** * Read file contents as a stream * @param path Absolute or relative path to file @@ -437,6 +408,15 @@ export interface Runtime { * @returns Result with new workspace path and source branch, or error */ forkWorkspace(params: WorkspaceForkParams): Promise; + + /** + * Get the runtime's temp directory (absolute path, resolved). + * - LocalRuntime: /tmp (or OS temp dir) + * - SSHRuntime: Resolved remote temp dir (e.g., /tmp) + * + * Used for background process output, temporary files, etc. + */ + tempDir(): Promise; } /** diff --git a/src/node/runtime/SSHBackgroundHandle.ts b/src/node/runtime/SSHBackgroundHandle.ts deleted file mode 100644 index 4c36c11edd..0000000000 --- a/src/node/runtime/SSHBackgroundHandle.ts +++ /dev/null @@ -1,108 +0,0 @@ -import type { BackgroundHandle } from "./Runtime"; -import type { SSHRuntime } from "./SSHRuntime"; -import { execBuffered } from "@/node/utils/runtime/helpers"; -import { expandTildeForSSH } from "./tildeExpansion"; -import { log } from "@/node/services/log"; -import { buildTerminateCommand, parseExitCode } from "./backgroundCommands"; - -/** - * Handle to an SSH background process. - * - * Uses file-based status detection: - * - Process is running if exit_code file doesn't exist (getExitCode returns null) - * - Exit code is read from exit_code file (written by trap on process exit) - * - * Output files (stdout.log, stderr.log) are on the remote machine - * and read by agents via bash("tail ...") commands. - */ -export class SSHBackgroundHandle implements BackgroundHandle { - private terminated = false; - - constructor( - private readonly sshRuntime: SSHRuntime, - private readonly pid: number, - /** Remote path to output directory (e.g., /tmp/mux-bashes/workspace/bg-xxx) */ - public readonly outputDir: string - ) {} - - /** - * Get the exit code from the remote exit_code file. - * Returns null if process is still running (file doesn't exist yet). - */ - async getExitCode(): Promise { - try { - const exitCodePath = expandTildeForSSH(`${this.outputDir}/exit_code`); - const result = await execBuffered( - this.sshRuntime, - `cat ${exitCodePath} 2>/dev/null || echo ""`, - { - cwd: "/", - timeout: 10, - } - ); - return parseExitCode(result.stdout); - } catch (error) { - log.debug( - `SSHBackgroundHandle.getExitCode: Error reading exit code: ${error instanceof Error ? error.message : String(error)}` - ); - return null; - } - } - - /** - * Terminate the process group via SSH. - * Sends SIGTERM to process group, waits briefly, then SIGKILL if still running. - * - * Uses negative PID to kill entire process group (PID === PGID due to set -m). - * Same pattern as Local for parity. - */ - async terminate(): Promise { - if (this.terminated) return; - - try { - // Use shared buildTerminateCommand for parity with Local - // Pass raw path + expandTildeForSSH to avoid double-quoting - // (expandTildeForSSH returns quoted strings, buildTerminateCommand would quote again) - const exitCodePath = `${this.outputDir}/exit_code`; - const terminateCmd = buildTerminateCommand(this.pid, exitCodePath, expandTildeForSSH); - await execBuffered(this.sshRuntime, terminateCmd, { - cwd: "/", - timeout: 15, - }); - log.debug(`SSHBackgroundHandle: Terminated process group ${this.pid}`); - } catch (error) { - // Process may already be dead - that's fine - log.debug( - `SSHBackgroundHandle.terminate: Error during terminate: ${error instanceof Error ? error.message : String(error)}` - ); - } - - this.terminated = true; - } - - /** - * Clean up resources. - * No local resources to clean for SSH handles. - */ - async dispose(): Promise { - // No local resources to clean up - } - - /** - * Write meta.json to the output directory on the remote machine. - */ - async writeMeta(metaJson: string): Promise { - try { - // Use heredoc for safe JSON writing - const metaPath = expandTildeForSSH(`${this.outputDir}/meta.json`); - await execBuffered(this.sshRuntime, `cat > ${metaPath} << 'METAEOF'\n${metaJson}\nMETAEOF`, { - cwd: "/", - timeout: 10, - }); - } catch (error) { - log.debug( - `SSHBackgroundHandle.writeMeta: Error: ${error instanceof Error ? error.message : String(error)}` - ); - } - } -} diff --git a/src/node/runtime/SSHRuntime.ts b/src/node/runtime/SSHRuntime.ts index 5c57ad8199..9e28919132 100644 --- a/src/node/runtime/SSHRuntime.ts +++ b/src/node/runtime/SSHRuntime.ts @@ -1,7 +1,6 @@ import { spawn } from "child_process"; import { Readable, Writable } from "stream"; import * as path from "path"; -import { randomBytes } from "crypto"; import type { Runtime, ExecOptions, @@ -14,8 +13,6 @@ import type { WorkspaceForkParams, WorkspaceForkResult, InitLogger, - BackgroundSpawnOptions, - BackgroundSpawnResult, } from "./Runtime"; import { RuntimeError as RuntimeErrorClass } from "./Runtime"; import { EXIT_CODE_ABORTED, EXIT_CODE_TIMEOUT } from "@/common/constants/exitCodes"; @@ -24,14 +21,11 @@ import { checkInitHookExists, createLineBufferedLoggers, getMuxEnv } from "./ini import { NON_INTERACTIVE_ENV_VARS } from "@/common/constants/env"; import { streamProcessToLogger } from "./streamProcess"; import { expandTildeForSSH, cdCommandForSSH } from "./tildeExpansion"; -import { getProjectName } from "@/node/utils/runtime/helpers"; +import { getProjectName, execBuffered } from "@/node/utils/runtime/helpers"; import { getErrorMessage } from "@/common/utils/errors"; import { execAsync, DisposableProcess } from "@/node/utils/disposableExec"; import { getControlPath } from "./sshConnectionPool"; import { getBashPath } from "@/node/utils/main/bashPath"; -import { SSHBackgroundHandle } from "./SSHBackgroundHandle"; -import { execBuffered } from "@/node/utils/runtime/helpers"; -import { shellQuote, buildSpawnCommand, parsePid } from "./backgroundCommands"; /** * Shell-escape helper for remote bash. @@ -96,8 +90,9 @@ export class SSHRuntime implements Runtime { /** * Get resolved background output directory (tilde expanded), caching the result. * This ensures all background process paths are absolute from the start. + * Public for use by BackgroundProcessExecutor. */ - private async getBgOutputDir(): Promise { + async getBgOutputDir(): Promise { if (this.resolvedBgOutputDir !== null) { return this.resolvedBgOutputDir; } @@ -271,131 +266,6 @@ export class SSHRuntime implements Runtime { return { stdout, stderr, stdin, exitCode, duration }; } - /** - * Spawn a background process on the remote machine. - * - * Uses nohup + shell redirection to detach the process from SSH. - * Exit code is captured via bash trap and written to exit_code file. - * Output is written directly to stdout.log and stderr.log on the remote. - * - * Output directory: {bgOutputDir}/{workspaceId}/{processId}/ - */ - async spawnBackground( - script: string, - options: BackgroundSpawnOptions - ): Promise { - log.debug(`SSHRuntime.spawnBackground: Spawning in ${options.cwd}`); - - // Verify working directory exists on remote (parity with local runtime) - const cwdCheck = await execBuffered(this, cdCommandForSSH(options.cwd), { - cwd: "/", - timeout: 10, - }); - if (cwdCheck.exitCode !== 0) { - return { success: false, error: `Working directory does not exist: ${options.cwd}` }; - } - - // Generate unique process ID and compute output directory - // /tmp is cleaned by OS, so no explicit cleanup needed - const processId = `bg-${randomBytes(4).toString("hex")}`; - const bgOutputDir = await this.getBgOutputDir(); - const outputDir = `${bgOutputDir}/${options.workspaceId}/${processId}`; - const stdoutPath = `${outputDir}/stdout.log`; - const stderrPath = `${outputDir}/stderr.log`; - const exitCodePath = `${outputDir}/exit_code`; - - // Use expandTildeForSSH for paths that may contain ~ (shescape.quote prevents tilde expansion) - const outputDirExpanded = expandTildeForSSH(outputDir); - const stdoutPathExpanded = expandTildeForSSH(stdoutPath); - const stderrPathExpanded = expandTildeForSSH(stderrPath); - const exitCodePathExpanded = expandTildeForSSH(exitCodePath); - - // Create output directory and empty files on remote - const mkdirResult = await execBuffered( - this, - `mkdir -p ${outputDirExpanded} && touch ${stdoutPathExpanded} ${stderrPathExpanded}`, - { cwd: "/", timeout: 30 } - ); - if (mkdirResult.exitCode !== 0) { - return { - success: false, - error: `Failed to create output directory: ${mkdirResult.stderr}`, - }; - } - - // Build the wrapper script with trap to capture exit code - // The trap writes exit code to file when the script exits (any exit path) - // Note: SSH uses expandTildeForSSH/cdCommandForSSH for tilde expansion, so we can't - // use buildWrapperScript directly. But we use buildSpawnCommand for parity. - const wrapperParts: string[] = []; - - // Set up trap first (use expanded path for tilde support) - wrapperParts.push(`trap 'echo $? > ${exitCodePathExpanded}' EXIT`); - - // Change to working directory - wrapperParts.push(cdCommandForSSH(options.cwd)); - - // Add environment variable exports (use shellQuote for parity with Local) - const envVars = { ...options.env, ...NON_INTERACTIVE_ENV_VARS }; - for (const [key, value] of Object.entries(envVars)) { - wrapperParts.push(`export ${key}=${shellQuote(value)}`); - } - - // Add the actual script - wrapperParts.push(script); - - const wrapperScript = wrapperParts.join(" && "); - - // Use shared buildSpawnCommand for parity with Local - // Use expandTildeForSSH for path quoting to support ~/... paths - const spawnCommand = buildSpawnCommand({ - wrapperScript, - stdoutPath, - stderrPath, - niceness: options.niceness, - quotePath: expandTildeForSSH, - }); - - try { - // No timeout - the spawn command backgrounds the process and returns immediately, - // but if wrapped in `timeout`, it would wait for the backgrounded process to exit. - // SSH connection hangs are protected by ConnectTimeout (see buildSshArgs in this file). - const result = await execBuffered(this, spawnCommand, { - cwd: "/", // cwd doesn't matter, we cd in the wrapper - }); - - if (result.exitCode !== 0) { - log.debug(`SSHRuntime.spawnBackground: spawn command failed: ${result.stderr}`); - return { - success: false, - error: `Failed to spawn background process: ${result.stderr}`, - }; - } - - const pid = parsePid(result.stdout); - if (!pid) { - log.debug(`SSHRuntime.spawnBackground: Invalid PID: ${result.stdout}`); - return { - success: false, - error: `Failed to get valid PID from spawn: ${result.stdout}`, - }; - } - - log.debug(`SSHRuntime.spawnBackground: Spawned with PID ${pid}`); - - // outputDir is already absolute (getBgOutputDir resolves tildes upfront) - const handle = new SSHBackgroundHandle(this, pid, outputDir); - return { success: true, handle, pid }; - } catch (error) { - const errorMessage = error instanceof Error ? error.message : String(error); - log.debug(`SSHRuntime.spawnBackground: Error: ${errorMessage}`); - return { - success: false, - error: `Failed to spawn background process: ${errorMessage}`, - }; - } - } - /** * Read file contents over SSH as a stream */ @@ -1425,6 +1295,15 @@ export class SSHRuntime implements Runtime { error: "Forking SSH workspaces is not yet implemented. Create a new workspace instead.", }); } + + /** + * Get the runtime's temp directory (resolved absolute path on remote). + */ + tempDir(): Promise { + // Use configured bgOutputDir's parent or default /tmp + // The bgOutputDir is typically /tmp/mux-bashes, so we return /tmp + return Promise.resolve("/tmp"); + } } /** diff --git a/src/node/runtime/WorktreeRuntime.test.ts b/src/node/runtime/WorktreeRuntime.test.ts index ad45a49fc2..949b309d59 100644 --- a/src/node/runtime/WorktreeRuntime.test.ts +++ b/src/node/runtime/WorktreeRuntime.test.ts @@ -5,7 +5,7 @@ import { WorktreeRuntime } from "./WorktreeRuntime"; describe("WorktreeRuntime constructor", () => { it("should expand tilde in srcBaseDir", () => { - const runtime = new WorktreeRuntime("~/workspace", "/tmp/bg"); + const runtime = new WorktreeRuntime("~/workspace"); const workspacePath = runtime.getWorkspacePath("/home/user/project", "branch"); // The workspace path should use the expanded home directory @@ -14,7 +14,7 @@ describe("WorktreeRuntime constructor", () => { }); it("should handle absolute paths without expansion", () => { - const runtime = new WorktreeRuntime("/absolute/path", "/tmp/bg"); + const runtime = new WorktreeRuntime("/absolute/path"); const workspacePath = runtime.getWorkspacePath("/home/user/project", "branch"); const expected = path.join("/absolute/path", "project", "branch"); @@ -22,7 +22,7 @@ describe("WorktreeRuntime constructor", () => { }); it("should handle bare tilde", () => { - const runtime = new WorktreeRuntime("~", "/tmp/bg"); + const runtime = new WorktreeRuntime("~"); const workspacePath = runtime.getWorkspacePath("/home/user/project", "branch"); const expected = path.join(os.homedir(), "project", "branch"); @@ -32,13 +32,13 @@ describe("WorktreeRuntime constructor", () => { describe("WorktreeRuntime.resolvePath", () => { it("should expand tilde to home directory", async () => { - const runtime = new WorktreeRuntime("/tmp", "/tmp/bg"); + const runtime = new WorktreeRuntime("/tmp"); const resolved = await runtime.resolvePath("~"); expect(resolved).toBe(os.homedir()); }); it("should expand tilde with path", async () => { - const runtime = new WorktreeRuntime("/tmp", "/tmp/bg"); + const runtime = new WorktreeRuntime("/tmp"); // Use a path that likely exists (or use /tmp if ~ doesn't have subdirs) const resolved = await runtime.resolvePath("~/.."); const expected = path.dirname(os.homedir()); @@ -46,20 +46,20 @@ describe("WorktreeRuntime.resolvePath", () => { }); it("should resolve absolute paths", async () => { - const runtime = new WorktreeRuntime("/tmp", "/tmp/bg"); + const runtime = new WorktreeRuntime("/tmp"); const resolved = await runtime.resolvePath("/tmp"); expect(resolved).toBe("/tmp"); }); it("should resolve non-existent paths without checking existence", async () => { - const runtime = new WorktreeRuntime("/tmp", "/tmp/bg"); + const runtime = new WorktreeRuntime("/tmp"); const resolved = await runtime.resolvePath("/this/path/does/not/exist/12345"); // Should resolve to absolute path without checking if it exists expect(resolved).toBe("/this/path/does/not/exist/12345"); }); it("should resolve relative paths from cwd", async () => { - const runtime = new WorktreeRuntime("/tmp", "/tmp/bg"); + const runtime = new WorktreeRuntime("/tmp"); const resolved = await runtime.resolvePath("."); // Should resolve to absolute path expect(path.isAbsolute(resolved)).toBe(true); diff --git a/src/node/runtime/WorktreeRuntime.ts b/src/node/runtime/WorktreeRuntime.ts index b8559aa267..60d69c1231 100644 --- a/src/node/runtime/WorktreeRuntime.ts +++ b/src/node/runtime/WorktreeRuntime.ts @@ -30,8 +30,8 @@ import { toPosixPath } from "@/node/utils/paths"; export class WorktreeRuntime extends LocalBaseRuntime { private readonly srcBaseDir: string; - constructor(srcBaseDir: string, bgOutputDir: string) { - super(bgOutputDir); + constructor(srcBaseDir: string) { + super(); // Expand tilde to actual home directory path for local file system operations this.srcBaseDir = expandTilde(srcBaseDir); } diff --git a/src/node/runtime/backgroundCommands.test.ts b/src/node/runtime/backgroundCommands.test.ts index 0010b3bcc6..249427c59c 100644 --- a/src/node/runtime/backgroundCommands.test.ts +++ b/src/node/runtime/backgroundCommands.test.ts @@ -1,4 +1,6 @@ import { describe, it, expect } from "bun:test"; +import { execSync } from "child_process"; +import * as fs from "fs/promises"; import { shellQuote, buildWrapperScript, @@ -39,8 +41,9 @@ describe("backgroundCommands", () => { script: "echo hello", }); + // Double quotes around trap command allow nested single-quoted paths expect(result).toBe( - "trap 'echo $? > '/tmp/exit_code'' EXIT && cd '/home/user/project' && echo hello" + `trap "echo \\$? > '/tmp/exit_code'" EXIT && cd '/home/user/project' && echo hello` ); }); @@ -67,6 +70,36 @@ describe("backgroundCommands", () => { expect(result).toContain("'/home/user/my project'"); }); + it("produces valid bash when exit code path contains spaces", async () => { + // Regression test: spaces in process ID (display_name) caused invalid trap syntax + // The trap command needs to properly nest quoted paths + const testDir = `/tmp/PR Checks ${Date.now()}`; + const exitCodePath = `${testDir}/exit_code`; + + const result = buildWrapperScript({ + exitCodePath, + cwd: "/tmp", + script: "exit 42", + }); + + // The wrapper script should be valid bash - execute it and verify exit code is captured + await fs.mkdir(testDir, { recursive: true }); + // Ensure no stale file + await fs.rm(exitCodePath, { force: true }); + + try { + execSync(`bash -c ${shellQuote(result)}`, { stdio: "pipe" }); + } catch { + // Expected - script exits with 42 + } + + // The exit code file MUST exist if the trap worked correctly + const exitCode = await fs.readFile(exitCodePath, "utf-8"); + expect(exitCode.trim()).toBe("42"); + + await fs.rm(testDir, { recursive: true }); + }); + it("escapes single quotes in env values", () => { const result = buildWrapperScript({ exitCodePath: "/tmp/exit_code", @@ -80,16 +113,14 @@ describe("backgroundCommands", () => { }); describe("buildSpawnCommand", () => { - it("uses set -m, nohup, redirections, and echoes PID", () => { + it("uses set -m, nohup, unified output with 2>&1, and echoes PID", () => { const result = buildSpawnCommand({ wrapperScript: "echo hello", - stdoutPath: "/tmp/out.log", - stderrPath: "/tmp/err.log", + outputPath: "/tmp/output.log", }); expect(result).toMatch(/^\(set -m; nohup 'bash' -c /); - expect(result).toContain("> '/tmp/out.log'"); - expect(result).toContain("2> '/tmp/err.log'"); + expect(result).toContain("> '/tmp/output.log' 2>&1"); expect(result).toContain("< /dev/null"); expect(result).toContain("& echo $!)"); }); @@ -97,8 +128,7 @@ describe("backgroundCommands", () => { it("includes niceness prefix when provided", () => { const result = buildSpawnCommand({ wrapperScript: "echo hello", - stdoutPath: "/tmp/out", - stderrPath: "/tmp/err", + outputPath: "/tmp/output.log", niceness: 10, }); @@ -108,8 +138,7 @@ describe("backgroundCommands", () => { it("uses custom bash path (including paths with spaces)", () => { const result = buildSpawnCommand({ wrapperScript: "echo hello", - stdoutPath: "/tmp/out", - stderrPath: "/tmp/err", + outputPath: "/tmp/output.log", bashPath: "/c/Program Files/Git/bin/bash.exe", }); @@ -119,8 +148,7 @@ describe("backgroundCommands", () => { it("quotes the wrapper script", () => { const result = buildSpawnCommand({ wrapperScript: "echo 'hello world'", - stdoutPath: "/tmp/out", - stderrPath: "/tmp/err", + outputPath: "/tmp/output.log", }); expect(result).toContain("-c 'echo '\"'\"'hello world'\"'\"''"); diff --git a/src/node/runtime/backgroundCommands.ts b/src/node/runtime/backgroundCommands.ts index 6e1bbb8343..8212bca0d6 100644 --- a/src/node/runtime/backgroundCommands.ts +++ b/src/node/runtime/backgroundCommands.ts @@ -57,7 +57,8 @@ export function buildWrapperScript(options: WrapperScriptOptions): string { const parts: string[] = []; // Set up trap first to capture exit code - parts.push(`trap 'echo $? > ${shellQuote(options.exitCodePath)}' EXIT`); + // Use double quotes for the trap command to allow nested single-quoted paths + parts.push(`trap "echo \\$? > ${shellQuote(options.exitCodePath)}" EXIT`); // Change to working directory parts.push(`cd ${shellQuote(options.cwd)}`); @@ -81,10 +82,8 @@ export function buildWrapperScript(options: WrapperScriptOptions): string { export interface SpawnCommandOptions { /** The wrapper script to execute */ wrapperScript: string; - /** Path for stdout redirection */ - stdoutPath: string; - /** Path for stderr redirection */ - stderrPath: string; + /** Path for unified output (stdout + stderr) redirection */ + outputPath: string; /** Path to bash executable (defaults to "bash") */ bashPath?: string; /** Optional niceness value for process priority */ @@ -100,6 +99,8 @@ export interface SpawnCommandOptions { * set -m: enables job control so backgrounded process gets its own process group (PID === PGID) * nohup: ignores SIGHUP (survives terminal hangup) * + * stdout and stderr are merged into a single output file with 2>&1 for unified display. + * * Returns PID via echo. With set -m, PID === PGID (process is its own group leader). */ export function buildSpawnCommand(options: SpawnCommandOptions): string { @@ -109,8 +110,7 @@ export function buildSpawnCommand(options: SpawnCommandOptions): string { return ( `(set -m; ${nicePrefix}nohup ${shellQuote(bash)} -c ${shellQuote(options.wrapperScript)} ` + - `> ${quotePath(options.stdoutPath)} ` + - `2> ${quotePath(options.stderrPath)} ` + + `> ${quotePath(options.outputPath)} 2>&1 ` + `< /dev/null & echo $!)` ); } diff --git a/src/node/runtime/runtimeFactory.ts b/src/node/runtime/runtimeFactory.ts index 61acab14c8..c3dfec2409 100644 --- a/src/node/runtime/runtimeFactory.ts +++ b/src/node/runtime/runtimeFactory.ts @@ -1,6 +1,3 @@ -import * as os from "os"; -import * as path from "path"; - import type { Runtime } from "./Runtime"; import { LocalRuntime } from "./LocalRuntime"; import { WorktreeRuntime } from "./WorktreeRuntime"; @@ -12,18 +9,6 @@ import { isIncompatibleRuntimeConfig } from "@/common/utils/runtimeCompatibility // Re-export for backward compatibility with existing imports export { isIncompatibleRuntimeConfig }; -/** - * Get the default output directory for background processes. - * Uses os.tmpdir() for platform-appropriate temp directory. - * - * Returns native path format (Windows or POSIX) since this is used by Node.js - * filesystem APIs. Conversion to POSIX for Git Bash shell commands happens - * at command construction time via toPosixPath(). - */ -function getDefaultBgOutputDir(): string { - return path.join(os.tmpdir(), "mux-bashes"); -} - /** * Error thrown when a workspace has an incompatible runtime configuration, * typically from a newer version of mux that added new runtime types. @@ -64,15 +49,13 @@ export function createRuntime(config: RuntimeConfig, options?: CreateRuntimeOpti ); } - const bgOutputDir = config.bgOutputDir ?? getDefaultBgOutputDir(); - switch (config.type) { case "local": // Check if this is legacy "local" with srcBaseDir (= worktree semantics) // or new "local" without srcBaseDir (= project-dir semantics) if (hasSrcBaseDir(config)) { // Legacy: "local" with srcBaseDir is treated as worktree - return new WorktreeRuntime(config.srcBaseDir, bgOutputDir); + return new WorktreeRuntime(config.srcBaseDir); } // Project-dir: uses project path directly, no isolation if (!options?.projectPath) { @@ -80,10 +63,10 @@ export function createRuntime(config: RuntimeConfig, options?: CreateRuntimeOpti "LocalRuntime requires projectPath in options for project-dir config (type: 'local' without srcBaseDir)" ); } - return new LocalRuntime(options.projectPath, bgOutputDir); + return new LocalRuntime(options.projectPath); case "worktree": - return new WorktreeRuntime(config.srcBaseDir, bgOutputDir); + return new WorktreeRuntime(config.srcBaseDir); case "ssh": return new SSHRuntime({ diff --git a/src/node/services/aiService.ts b/src/node/services/aiService.ts index e4353f67c6..15040aa861 100644 --- a/src/node/services/aiService.ts +++ b/src/node/services/aiService.ts @@ -1087,8 +1087,7 @@ export class AIService extends EventEmitter { runtime, // Plan files are always local - create a local runtime for plan file I/O // even when the workspace uses SSH runtime - localRuntime: - mode === "plan" ? new LocalRuntime(workspacePath, runtimeTempDir) : undefined, + localRuntime: mode === "plan" ? new LocalRuntime(workspacePath) : undefined, secrets: secretsToRecord(projectSecrets), muxEnv: getMuxEnv( metadata.projectPath, diff --git a/src/node/services/backgroundProcessExecutor.ts b/src/node/services/backgroundProcessExecutor.ts new file mode 100644 index 0000000000..85ee90d6ed --- /dev/null +++ b/src/node/services/backgroundProcessExecutor.ts @@ -0,0 +1,559 @@ +/** + * Unified executor for background bash processes. + * + * ALL bash commands are spawned through this executor with background-style + * infrastructure (nohup, file output, exit code trap). This enables: + * + * 1. Uniform code path - one spawn mechanism for all bash commands + * 2. Crash resilience - output always persisted to files + * 3. Seamless fgβ†’bg transition - "background this" = "stop waiting" + * + * Uses runtime.tempDir() for runtime-agnostic temp directory resolution. + * Works identically for local and SSH runtimes. + */ + +import type { Runtime, BackgroundHandle, ExecStream } from "@/node/runtime/Runtime"; +import * as fs from "fs/promises"; +import * as path from "path"; +import { log } from "./log"; +import { + buildWrapperScript, + buildSpawnCommand, + parsePid, + parseExitCode, + buildTerminateCommand, + shellQuote, +} from "@/node/runtime/backgroundCommands"; +import { execBuffered } from "@/node/utils/runtime/helpers"; +import { NON_INTERACTIVE_ENV_VARS } from "@/common/constants/env"; +import { toPosixPath } from "@/node/utils/paths"; + +/** + * Quote a path for shell commands. + * On Windows, first converts to POSIX format, then shell-quotes. + * On Unix, just shell-quotes (handles spaces, special chars). + */ +function quotePathForShell(p: string): string { + const posixPath = toPosixPath(p); + return shellQuote(posixPath); +} + +/** Safe fallback cwd for exec calls - /tmp exists on all POSIX systems (including WSL/Git Bash) */ +const FALLBACK_CWD = "/tmp"; + +/** Helper to extract error message for logging */ +function errorMsg(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +/** Subdirectory under temp for background process output */ +export const BG_OUTPUT_SUBDIR = "mux-bashes"; + +/** Output filename for combined stdout/stderr */ +export const OUTPUT_FILENAME = "output.log"; + +/** Exit code filename */ +export const EXIT_CODE_FILENAME = "exit_code"; + +/** + * Compute paths for a background process output directory. + * @param bgOutputDir Base directory (e.g., /tmp/mux-bashes or ~/.mux/sessions) + * @param workspaceId Workspace identifier + * @param processId Process identifier + */ +export function computeOutputPaths( + bgOutputDir: string, + workspaceId: string, + processId: string +): { outputDir: string; outputPath: string; exitCodePath: string } { + const outputDir = `${bgOutputDir}/${workspaceId}/${processId}`; + return { + outputDir, + outputPath: `${outputDir}/${OUTPUT_FILENAME}`, + exitCodePath: `${outputDir}/${EXIT_CODE_FILENAME}`, + }; +} + +/** + * Options for spawning a process + */ +export interface SpawnOptions { + /** Working directory for command execution */ + cwd: string; + /** Workspace ID for output directory organization */ + workspaceId: string; + /** Process ID (e.g., "bash_1") - generated by BackgroundProcessManager */ + processId: string; + /** Environment variables to inject */ + env?: Record; + /** Process niceness level (-20 to 19, lower = higher priority) */ + niceness?: number; +} + +/** + * Result of spawning a process + */ +export type SpawnResult = + | { success: true; handle: BackgroundHandle; pid: number; outputDir: string } + | { success: false; error: string }; + +/** + * Spawn a background process using runtime.exec (works for both local and SSH). + * + * All processes get the same infrastructure: + * - nohup/setsid for process isolation + * - stdout/stderr merged into single output.log with 2>&1 + * - Exit code captured via bash trap + * + * Uses runtime.tempDir() for output directory, making the code runtime-agnostic. + * + * @param runtime Runtime to spawn on + * @param script Script to execute + * @param options Spawn options + */ +export async function spawnProcess( + runtime: Runtime, + script: string, + options: SpawnOptions +): Promise { + log.debug(`BackgroundProcessExecutor.spawnProcess: Spawning in ${options.cwd}`); + + // Get temp directory from runtime (absolute path, runtime-agnostic) + const tempDir = await runtime.tempDir(); + const bgOutputDir = `${tempDir}/${BG_OUTPUT_SUBDIR}`; + + // Use shell-safe quoting for paths (handles spaces, special chars) + const quotePath = quotePathForShell; + + // Verify working directory exists + const cwdCheck = await execBuffered(runtime, `cd ${quotePath(options.cwd)}`, { + cwd: FALLBACK_CWD, + timeout: 10, + }); + if (cwdCheck.exitCode !== 0) { + return { success: false, error: `Working directory does not exist: ${options.cwd}` }; + } + + // Compute output paths (unified output.log instead of separate stdout/stderr) + const { outputDir, outputPath, exitCodePath } = computeOutputPaths( + bgOutputDir, + options.workspaceId, + options.processId + ); + + // Create output directory and empty file + const mkdirResult = await execBuffered( + runtime, + `mkdir -p ${quotePath(outputDir)} && touch ${quotePath(outputPath)}`, + { cwd: FALLBACK_CWD, timeout: 30 } + ); + if (mkdirResult.exitCode !== 0) { + return { + success: false, + error: `Failed to create output directory: ${mkdirResult.stderr}`, + }; + } + + // Build wrapper script (same for all runtimes now that paths are absolute) + // Note: buildWrapperScript handles quoting internally via shellQuote + const wrapperScript = buildWrapperScript({ + exitCodePath, + cwd: options.cwd, + env: { ...options.env, ...NON_INTERACTIVE_ENV_VARS }, + script, + }); + + const spawnCommand = buildSpawnCommand({ + wrapperScript, + outputPath, + niceness: options.niceness, + quotePath, + }); + + try { + // No timeout - the spawn command backgrounds the process and returns immediately + const result = await execBuffered(runtime, spawnCommand, { + cwd: FALLBACK_CWD, + }); + + if (result.exitCode !== 0) { + log.debug(`BackgroundProcessExecutor.spawnProcess: spawn command failed: ${result.stderr}`); + return { + success: false, + error: `Failed to spawn background process: ${result.stderr}`, + }; + } + + const pid = parsePid(result.stdout); + if (!pid) { + log.debug(`BackgroundProcessExecutor.spawnProcess: Invalid PID: ${result.stdout}`); + return { + success: false, + error: `Failed to get valid PID from spawn: ${result.stdout}`, + }; + } + + log.debug(`BackgroundProcessExecutor.spawnProcess: Spawned with PID ${pid}`); + const handle = new RuntimeBackgroundHandle(runtime, pid, outputDir, quotePath); + return { success: true, handle, pid, outputDir }; + } catch (error) { + const errorMessage = errorMsg(error); + log.debug(`BackgroundProcessExecutor.spawnProcess: Error: ${errorMessage}`); + return { + success: false, + error: `Failed to spawn background process: ${errorMessage}`, + }; + } +} + +/** + * Unified handle to a background process. + * Uses runtime.exec for all operations, working identically for local and SSH. + * + * Output files (output.log, exit_code) are on the runtime's filesystem. + * This handle provides lifecycle management via execBuffered commands. + */ +class RuntimeBackgroundHandle implements BackgroundHandle { + private terminated = false; + + constructor( + private readonly runtime: Runtime, + private readonly pid: number, + public readonly outputDir: string, + private readonly quotePath: (p: string) => string + ) {} + + /** + * Get the exit code from the exit_code file. + * Returns null if process is still running (file doesn't exist yet). + */ + async getExitCode(): Promise { + try { + const exitCodePath = this.quotePath(`${this.outputDir}/${EXIT_CODE_FILENAME}`); + const result = await execBuffered( + this.runtime, + `cat ${exitCodePath} 2>/dev/null || echo ""`, + { cwd: FALLBACK_CWD, timeout: 10 } + ); + return parseExitCode(result.stdout); + } catch (error) { + log.debug(`RuntimeBackgroundHandle.getExitCode: Error: ${errorMsg(error)}`); + return null; + } + } + + /** + * Terminate the process group. + * Sends SIGTERM to process group, waits briefly, then SIGKILL if still running. + */ + async terminate(): Promise { + if (this.terminated) return; + + try { + const exitCodePath = `${this.outputDir}/${EXIT_CODE_FILENAME}`; + const terminateCmd = buildTerminateCommand(this.pid, exitCodePath, this.quotePath); + await execBuffered(this.runtime, terminateCmd, { + cwd: FALLBACK_CWD, + timeout: 15, + }); + log.debug(`RuntimeBackgroundHandle: Terminated process group ${this.pid}`); + } catch (error) { + // Process may already be dead - that's fine + log.debug(`RuntimeBackgroundHandle.terminate: Error: ${errorMsg(error)}`); + } + + this.terminated = true; + } + + /** + * Clean up resources. + * No resources to clean - process runs independently via nohup. + */ + async dispose(): Promise { + // No resources to clean up + } + + /** + * Write meta.json to the output directory. + */ + async writeMeta(metaJson: string): Promise { + try { + const metaPath = this.quotePath(`${this.outputDir}/meta.json`); + await execBuffered(this.runtime, `cat > ${metaPath} << 'METAEOF'\n${metaJson}\nMETAEOF`, { + cwd: FALLBACK_CWD, + timeout: 10, + }); + } catch (error) { + log.debug(`RuntimeBackgroundHandle.writeMeta: Error: ${errorMsg(error)}`); + } + } + + /** + * Read output from output.log at the given byte offset. + * Uses tail -c to read from offset - works on both Linux and macOS. + */ + async readOutput(offset: number): Promise<{ content: string; newOffset: number }> { + try { + const filePath = this.quotePath(`${this.outputDir}/${OUTPUT_FILENAME}`); + // Get file size first to know how much we read + // Use wc -c - works on both Linux and macOS (unlike stat flags which differ) + const sizeResult = await execBuffered( + this.runtime, + `wc -c < ${filePath} 2>/dev/null || echo 0`, + { cwd: FALLBACK_CWD, timeout: 10 } + ); + const fileSize = parseInt(sizeResult.stdout.trim(), 10) || 0; + + if (offset >= fileSize) { + return { content: "", newOffset: offset }; + } + + // Read from offset to end of file using tail -c (faster than dd bs=1) + // tail -c +N means "start at byte N" (1-indexed) + const readResult = await execBuffered( + this.runtime, + `tail -c +${offset + 1} ${filePath} 2>/dev/null`, + { cwd: FALLBACK_CWD, timeout: 30 } + ); + + return { + content: readResult.stdout, + newOffset: offset + readResult.stdout.length, + }; + } catch (error) { + log.debug(`RuntimeBackgroundHandle.readOutput: Error: ${errorMsg(error)}`); + return { content: "", newOffset: offset }; + } + } +} + +/** + * Options for migrating a foreground process to background + */ +export interface MigrateOptions { + /** Working directory (for display in meta.json) */ + cwd: string; + /** Workspace ID */ + workspaceId: string; + /** Process ID (e.g., "bash_1") - generated by BackgroundProcessManager */ + processId: string; + /** Original script being executed */ + script: string; + /** Output already captured while running in foreground */ + existingOutput: string[]; + /** Human-readable name for the process */ + displayName?: string; +} + +/** + * Result of migrating a foreground process + */ +export type MigrateResult = + | { success: true; handle: BackgroundHandle; outputDir: string } + | { success: false; error: string }; + +/** + * Migrate a foreground process to background tracking. + * + * This is called when user clicks "Background" on a running foreground process. + * The process continues running, but we: + * 1. Create output directory and write existing output + * 2. Continue consuming streams and writing to unified output.log + * 3. Track exit code when process completes + * 4. Return a BackgroundHandle for the manager to track + * + * Note: Output files are written locally (not via runtime), so this works + * for SSH runtime where streams are already being piped to the local machine. + * + * @param execStream The running process's streams + * @param options Migration options + * @param bgOutputDir Base directory for output files + */ +export async function migrateToBackground( + execStream: ExecStream, + options: MigrateOptions, + bgOutputDir: string +): Promise { + // Use shared path computation (path.join for local filesystem) + const { outputDir, outputPath } = computeOutputPaths( + bgOutputDir, + options.workspaceId, + options.processId + ); + + try { + // Create output directory + await fs.mkdir(outputDir, { recursive: true }); + + // Write existing output to unified output.log + await fs.writeFile(outputPath, options.existingOutput.join("\n") + "\n"); + + // Create handle that will continue writing to file + const handle = new MigratedBackgroundHandle(execStream, outputDir, outputPath); + + // Start consuming remaining output in background + handle.startConsuming(); + + return { success: true, handle, outputDir }; + } catch (error) { + const errorMessage = errorMsg(error); + log.debug(`migrateToBackground: Error: ${errorMessage}`); + return { success: false, error: `Failed to migrate process: ${errorMessage}` }; + } +} + +/** + * Handle for a migrated foreground process. + * + * Unlike RuntimeBackgroundHandle which uses runtime.exec for file operations, + * this handle uses local filesystem directly because the streams are already + * being piped to the local machine (even for SSH runtime). + * + * Both stdout and stderr are written to a unified output.log file. + */ +class MigratedBackgroundHandle implements BackgroundHandle { + private exitCodeValue: number | null = null; + private consuming = false; + private outputFd: fs.FileHandle | null = null; + + constructor( + private readonly execStream: ExecStream, + public readonly outputDir: string, + private readonly outputPath: string + ) {} + + /** + * Start consuming remaining output from streams and writing to unified file. + * Called after handle is created to begin background file writing. + */ + startConsuming(): void { + if (this.consuming) return; + this.consuming = true; + + // Open output file once, consume both streams to it + void this.consumeStreams(); + + // Track exit code + void this.execStream.exitCode.then((code) => { + this.exitCodeValue = code; + // Write exit code to file + void this.writeExitCode(code); + }); + } + + /** + * Consume both stdout and stderr streams and append to unified output file. + */ + private async consumeStreams(): Promise { + try { + this.outputFd = await fs.open(this.outputPath, "a"); + + // Consume both streams concurrently, both writing to same file + await Promise.all([ + this.consumeStream(this.execStream.stdout), + this.consumeStream(this.execStream.stderr), + ]); + } catch (error) { + log.debug(`MigratedBackgroundHandle.consumeStreams: ${errorMsg(error)}`); + } finally { + if (this.outputFd) { + await this.outputFd.close(); + this.outputFd = null; + } + } + } + + /** + * Consume a stream and append to the shared output file. + */ + private async consumeStream(stream: ReadableStream): Promise { + try { + const reader = stream.getReader(); + + try { + while (true) { + const { value, done } = await reader.read(); + if (done) break; + if (value && this.outputFd) { + await this.outputFd.write(value); + } + } + } finally { + reader.releaseLock(); + } + } catch (error) { + // Stream may have been cancelled or process killed - that's fine + log.debug(`MigratedBackgroundHandle.consumeStream: ${errorMsg(error)}`); + } + } + + /** + * Write exit code to file. + */ + private async writeExitCode(code: number): Promise { + try { + const exitCodePath = path.join(this.outputDir, EXIT_CODE_FILENAME); + await fs.writeFile(exitCodePath, String(code)); + } catch (error) { + log.debug(`MigratedBackgroundHandle.writeExitCode: ${errorMsg(error)}`); + } + } + + getExitCode(): Promise { + return Promise.resolve(this.exitCodeValue); + } + + async terminate(): Promise { + // ExecStream doesn't expose a kill method directly + // Cancel the streams to stop reading (process continues but we stop tracking) + try { + await this.execStream.stdout.cancel(); + await this.execStream.stderr.cancel(); + } catch { + // Streams may already be closed + } + } + + async dispose(): Promise { + // Close any open file handles + await this.outputFd?.close().catch(() => { + /* ignore */ + }); + } + + async writeMeta(metaJson: string): Promise { + try { + const metaPath = path.join(this.outputDir, "meta.json"); + await fs.writeFile(metaPath, metaJson); + } catch (error) { + log.debug(`MigratedBackgroundHandle.writeMeta: ${errorMsg(error)}`); + } + } + + async readOutput(offset: number): Promise<{ content: string; newOffset: number }> { + try { + const stat = await fs.stat(this.outputPath); + const fileSize = stat.size; + + if (offset >= fileSize) { + return { content: "", newOffset: offset }; + } + + // Read from offset to end + const fd = await fs.open(this.outputPath, "r"); + try { + const buffer = Buffer.alloc(fileSize - offset); + const { bytesRead } = await fd.read(buffer, 0, buffer.length, offset); + return { + content: buffer.slice(0, bytesRead).toString("utf-8"), + newOffset: offset + bytesRead, + }; + } finally { + await fd.close(); + } + } catch (error) { + log.debug(`MigratedBackgroundHandle.readOutput: ${errorMsg(error)}`); + return { content: "", newOffset: offset }; + } + } +} diff --git a/src/node/services/backgroundProcessManager.test.ts b/src/node/services/backgroundProcessManager.test.ts index 23d0f775bf..903a5ea2d1 100644 --- a/src/node/services/backgroundProcessManager.test.ts +++ b/src/node/services/backgroundProcessManager.test.ts @@ -5,6 +5,10 @@ import type { Runtime } from "@/node/runtime/Runtime"; import * as fs from "fs/promises"; import * as path from "path"; import * as os from "os"; +import { createBashTool } from "@/node/services/tools/bash"; +import { createBashOutputTool } from "@/node/services/tools/bash_output"; +import { TestTempDir, createTestToolConfig } from "@/node/services/tools/testHelpers"; +import type { BashToolResult, BashOutputToolResult } from "@/common/types/tools"; describe("BackgroundProcessManager", () => { let manager: BackgroundProcessManager; @@ -16,30 +20,41 @@ describe("BackgroundProcessManager", () => { const testWorkspaceId2 = `test-ws2-${testRunId}`; beforeEach(async () => { - manager = new BackgroundProcessManager(); - // Create isolated temp directory for sessions + // Create isolated temp directory for each test to avoid cross-test pollution bgOutputDir = await fs.mkdtemp(path.join(os.tmpdir(), "bg-proc-test-")); - runtime = new LocalRuntime(process.cwd(), bgOutputDir); + manager = new BackgroundProcessManager(bgOutputDir); + runtime = new LocalRuntime(process.cwd()); }); afterEach(async () => { // Cleanup: terminate all processes await manager.cleanup(testWorkspaceId); await manager.cleanup(testWorkspaceId2); - // Remove temp sessions directory + // Remove temp sessions directory (legacy) await fs.rm(bgOutputDir, { recursive: true, force: true }).catch(() => undefined); + // Remove actual output directories from /tmp/mux-bashes (where executor writes) + await fs + .rm(`/tmp/mux-bashes/${testWorkspaceId}`, { recursive: true, force: true }) + .catch(() => undefined); + await fs + .rm(`/tmp/mux-bashes/${testWorkspaceId2}`, { recursive: true, force: true }) + .catch(() => undefined); }); describe("spawn", () => { it("should spawn a background process and return process ID and outputDir", async () => { + const displayName = `test-${Date.now()}`; const result = await manager.spawn(runtime, testWorkspaceId, "echo hello", { cwd: process.cwd(), + displayName, }); expect(result.success).toBe(true); if (result.success) { - expect(result.processId).toMatch(/^bg-/); - expect(result.outputDir).toContain(bgOutputDir); + // Process ID is now the display name directly + expect(result.processId).toBe(displayName); + // outputDir is now under runtime.tempDir()/mux-bashes// + expect(result.outputDir).toContain("mux-bashes"); expect(result.outputDir).toContain(testWorkspaceId); expect(result.outputDir).toContain(result.processId); } @@ -48,14 +63,16 @@ describe("BackgroundProcessManager", () => { it("should return error on spawn failure", async () => { const result = await manager.spawn(runtime, testWorkspaceId, "echo test", { cwd: "/nonexistent/path/that/does/not/exist", + displayName: "test", }); expect(result.success).toBe(false); }); - it("should write stdout and stderr to files", async () => { + it("should write stdout and stderr to unified output file", async () => { const result = await manager.spawn(runtime, testWorkspaceId, "echo hello; echo world >&2", { cwd: process.cwd(), + displayName: "test", }); expect(result.success).toBe(true); @@ -63,20 +80,19 @@ describe("BackgroundProcessManager", () => { // Wait a moment for output to be written await new Promise((resolve) => setTimeout(resolve, 100)); - const stdoutPath = path.join(result.outputDir, "stdout.log"); - const stderrPath = path.join(result.outputDir, "stderr.log"); + const outputPath = path.join(result.outputDir, "output.log"); + const output = await fs.readFile(outputPath, "utf-8"); - const stdout = await fs.readFile(stdoutPath, "utf-8"); - const stderr = await fs.readFile(stderrPath, "utf-8"); - - expect(stdout).toContain("hello"); - expect(stderr).toContain("world"); + // Both stdout and stderr go to the same file + expect(output).toContain("hello"); + expect(output).toContain("world"); } }); it("should write meta.json with process info", async () => { const result = await manager.spawn(runtime, testWorkspaceId, "echo test", { cwd: process.cwd(), + displayName: "test", }); expect(result.success).toBe(true); @@ -98,6 +114,7 @@ describe("BackgroundProcessManager", () => { it("should return process by ID", async () => { const spawnResult = await manager.spawn(runtime, testWorkspaceId, "sleep 1", { cwd: process.cwd(), + displayName: "test", }); if (spawnResult.success) { @@ -116,16 +133,30 @@ describe("BackgroundProcessManager", () => { describe("list", () => { it("should list all processes", async () => { - await manager.spawn(runtime, testWorkspaceId, "sleep 1", { cwd: process.cwd() }); - await manager.spawn(runtime, testWorkspaceId, "sleep 1", { cwd: process.cwd() }); + // Use unique display names since they're now used as process IDs + await manager.spawn(runtime, testWorkspaceId, "sleep 1", { + cwd: process.cwd(), + displayName: "test-list-1", + }); + await manager.spawn(runtime, testWorkspaceId, "sleep 1", { + cwd: process.cwd(), + displayName: "test-list-2", + }); const processes = await manager.list(); expect(processes.length).toBeGreaterThanOrEqual(2); }); it("should filter by workspace ID", async () => { - await manager.spawn(runtime, testWorkspaceId, "sleep 1", { cwd: process.cwd() }); - await manager.spawn(runtime, testWorkspaceId2, "sleep 1", { cwd: process.cwd() }); + // Use unique display names since they're now used as process IDs + await manager.spawn(runtime, testWorkspaceId, "sleep 1", { + cwd: process.cwd(), + displayName: "test-filter-ws1", + }); + await manager.spawn(runtime, testWorkspaceId2, "sleep 1", { + cwd: process.cwd(), + displayName: "test-filter-ws2", + }); const ws1Processes = await manager.list(testWorkspaceId); const ws2Processes = await manager.list(testWorkspaceId2); @@ -141,6 +172,7 @@ describe("BackgroundProcessManager", () => { it("should terminate a running process", async () => { const spawnResult = await manager.spawn(runtime, testWorkspaceId, "sleep 10", { cwd: process.cwd(), + displayName: "test", }); if (spawnResult.success) { @@ -160,6 +192,7 @@ describe("BackgroundProcessManager", () => { it("should be idempotent (double-terminate succeeds)", async () => { const spawnResult = await manager.spawn(runtime, testWorkspaceId, "sleep 10", { cwd: process.cwd(), + displayName: "test", }); if (spawnResult.success) { @@ -176,11 +209,16 @@ describe("BackgroundProcessManager", () => { it("should kill all processes for a workspace and remove from memory", async () => { await manager.spawn(runtime, testWorkspaceId, "sleep 10", { cwd: process.cwd(), + displayName: "test", }); await manager.spawn(runtime, testWorkspaceId, "sleep 10", { cwd: process.cwd(), + displayName: "test", + }); + await manager.spawn(runtime, testWorkspaceId2, "sleep 10", { + cwd: process.cwd(), + displayName: "test", }); - await manager.spawn(runtime, testWorkspaceId2, "sleep 10", { cwd: process.cwd() }); await manager.cleanup(testWorkspaceId); @@ -196,12 +234,14 @@ describe("BackgroundProcessManager", () => { describe("terminateAll", () => { it("should kill all processes across all workspaces", async () => { - // Spawn processes in multiple workspaces + // Spawn processes in multiple workspaces (unique display names since they're process IDs) await manager.spawn(runtime, testWorkspaceId, "sleep 10", { cwd: process.cwd(), + displayName: "test-termall-ws1", }); await manager.spawn(runtime, testWorkspaceId2, "sleep 10", { cwd: process.cwd(), + displayName: "test-termall-ws2", }); // Verify both workspaces have running processes @@ -236,6 +276,7 @@ describe("BackgroundProcessManager", () => { it("should track process exit and update meta.json", async () => { const result = await manager.spawn(runtime, testWorkspaceId, "exit 42", { cwd: process.cwd(), + displayName: "test", }); if (result.success) { @@ -259,6 +300,7 @@ describe("BackgroundProcessManager", () => { it("should keep output files after process exits", async () => { const result = await manager.spawn(runtime, testWorkspaceId, "echo test; exit 0", { cwd: process.cwd(), + displayName: "test", }); if (result.success) { @@ -267,10 +309,10 @@ describe("BackgroundProcessManager", () => { const proc = await manager.getProcess(result.processId); expect(proc?.status).toBe("exited"); - // Verify stdout file still contains output - const stdoutPath = path.join(result.outputDir, "stdout.log"); - const stdout = await fs.readFile(stdoutPath, "utf-8"); - expect(stdout).toContain("test"); + // Verify output file still contains output + const outputPath = path.join(result.outputDir, "output.log"); + const output = await fs.readFile(outputPath, "utf-8"); + expect(output).toContain("test"); } }); @@ -278,6 +320,7 @@ describe("BackgroundProcessManager", () => { // Spawn a long-running process const result = await manager.spawn(runtime, testWorkspaceId, "sleep 60", { cwd: process.cwd(), + displayName: "test", }); if (result.success) { @@ -294,6 +337,7 @@ describe("BackgroundProcessManager", () => { // Spawn a long-running process const result = await manager.spawn(runtime, testWorkspaceId, "sleep 60", { cwd: process.cwd(), + displayName: "test", }); if (result.success) { @@ -319,6 +363,7 @@ describe("BackgroundProcessManager", () => { // This creates: parent bash -> child sleep const result = await manager.spawn(runtime, testWorkspaceId, "bash -c 'sleep 60 & wait'", { cwd: process.cwd(), + displayName: "test", }); expect(result.success).toBe(true); @@ -350,10 +395,291 @@ describe("BackgroundProcessManager", () => { }); }); + describe("getOutput", () => { + it("should return stdout from a running process", async () => { + // Spawn a process that writes output over time + // Use longer sleep and explicit flush to ensure output is written to file + const result = await manager.spawn( + runtime, + testWorkspaceId, + "echo 'line 1'; sleep 0.3; echo 'line 2'", + { cwd: process.cwd(), displayName: "test" } + ); + + expect(result.success).toBe(true); + if (!result.success) return; + + // Wait for first line to be written and flushed (increased for CI reliability) + await new Promise((resolve) => setTimeout(resolve, 200)); + + // Get output - should have at least the first line + const output1 = await manager.getOutput(result.processId); + expect(output1.success).toBe(true); + if (!output1.success) return; + + expect(output1.output).toContain("line 1"); + + // Wait for second line (sleep 0.3s + buffer for CI) + await new Promise((resolve) => setTimeout(resolve, 500)); + + // Get output again - should have incremental output (line 2) + const output2 = await manager.getOutput(result.processId); + expect(output2.success).toBe(true); + if (!output2.success) return; + + // Second call should only return new content (line 2) + expect(output2.output).toContain("line 2"); + // And should NOT contain line 1 again (incremental reads) + expect(output2.output).not.toContain("line 1"); + }); + + it("should return stderr from a running process", async () => { + const result = await manager.spawn(runtime, testWorkspaceId, "echo 'error message' >&2", { + cwd: process.cwd(), + displayName: "test", + }); + + expect(result.success).toBe(true); + if (!result.success) return; + + await new Promise((resolve) => setTimeout(resolve, 100)); + + const output = await manager.getOutput(result.processId); + expect(output.success).toBe(true); + if (!output.success) return; + + expect(output.output).toContain("error message"); + }); + + it("should return error for non-existent process", async () => { + const output = await manager.getOutput("bash_nonexistent"); + expect(output.success).toBe(false); + if (output.success) return; + expect(output.error).toContain("not found"); + }); + + it("should return correct status for running vs exited process", async () => { + const result = await manager.spawn(runtime, testWorkspaceId, "echo done; exit 0", { + cwd: process.cwd(), + displayName: "test", + }); + + expect(result.success).toBe(true); + if (!result.success) return; + + // Immediately should be running + const output1 = await manager.getOutput(result.processId); + expect(output1.success).toBe(true); + if (!output1.success) return; + // Status could be running or already exited depending on timing + + // Wait for exit + await new Promise((resolve) => setTimeout(resolve, 200)); + + const output2 = await manager.getOutput(result.processId); + expect(output2.success).toBe(true); + if (!output2.success) return; + expect(output2.status).toBe("exited"); + expect(output2.exitCode).toBe(0); + }); + + it("should filter output with regex when provided", async () => { + const result = await manager.spawn( + runtime, + testWorkspaceId, + "echo 'INFO: message'; echo 'DEBUG: noise'; echo 'INFO: another'", + { cwd: process.cwd(), displayName: "test" } + ); + + expect(result.success).toBe(true); + if (!result.success) return; + + await new Promise((resolve) => setTimeout(resolve, 100)); + + // Filter for INFO lines only + const output = await manager.getOutput(result.processId, "INFO"); + expect(output.success).toBe(true); + if (!output.success) return; + + expect(output.output).toContain("INFO: message"); + expect(output.output).toContain("INFO: another"); + expect(output.output).not.toContain("DEBUG"); + }); + }); + + describe("integration: spawn and getOutput", () => { + it("should retrieve output after spawn using same manager instance", async () => { + // This test verifies the core workflow: spawn -> getOutput + // Both must use the SAME manager instance + + // Spawn process that produces output + const result = await manager.spawn(runtime, testWorkspaceId, "echo 'hello from bg'", { + cwd: process.cwd(), + displayName: "test", + }); + + expect(result.success).toBe(true); + if (!result.success) return; + + // Wait for output + await new Promise((resolve) => setTimeout(resolve, 100)); + + // Verify we can read output using the SAME manager + const output = await manager.getOutput(result.processId); + expect(output.success).toBe(true); + if (!output.success) return; + + expect(output.output).toContain("hello from bg"); + }); + + it("should read from offset 0 on first call even if file already has content", async () => { + // Spawn a process that writes output immediately + const result = await manager.spawn(runtime, testWorkspaceId, "echo 'initial output'", { + cwd: process.cwd(), + displayName: "test", + }); + + expect(result.success).toBe(true); + if (!result.success) return; + + // Wait longer to ensure output is definitely written + await new Promise((resolve) => setTimeout(resolve, 500)); + + // Verify the file has content + const outputPath = path.join(result.outputDir, "output.log"); + const fileContent = await fs.readFile(outputPath, "utf-8"); + expect(fileContent).toContain("initial output"); + + // Now call getOutput - first call should read from offset 0 + const output = await manager.getOutput(result.processId); + expect(output.success).toBe(true); + if (!output.success) return; + + // Should have the output even though some time has passed + expect(output.output).toContain("initial output"); + }); + + it("DEBUG: verifies outputDir from spawn matches getProcess", async () => { + // Verify that outputDir returned from spawn is the same as what getProcess returns + const result = await manager.spawn(runtime, testWorkspaceId, "echo 'verify test'", { + cwd: process.cwd(), + displayName: "test", + }); + + expect(result.success).toBe(true); + if (!result.success) return; + + const proc = await manager.getProcess(result.processId); + expect(proc).not.toBeNull(); + + // CRITICAL: outputDir from spawn MUST match outputDir from getProcess + expect(proc!.outputDir).toBe(result.outputDir); + + // Wait for output to be written + await new Promise((resolve) => setTimeout(resolve, 100)); + + // Verify file exists at the expected path + const outputPath = path.join(result.outputDir, "output.log"); + const content = await fs.readFile(outputPath, "utf-8"); + expect(content).toContain("verify test"); + + // Now getOutput should return the content + const output = await manager.getOutput(result.processId); + expect(output.success).toBe(true); + if (output.success) { + expect(output.output).toContain("verify test"); + } + }); + + it("should work when spawned via bash tool and read via bash_output tool", async () => { + // This simulates the exact flow in the real system: + // 1. bash tool with run_in_background=true spawns process + // 2. bash_output tool reads output + + const tempDir = new TestTempDir("test-bg-integration"); + + // Create shared config with the SAME manager instance + const config = createTestToolConfig(tempDir.path, { + workspaceId: testWorkspaceId, + sessionsDir: tempDir.path, + }); + config.backgroundProcessManager = manager; + config.runtime = runtime; + + // Create bash tool and spawn background process + const bashTool = createBashTool(config); + const spawnResult = (await bashTool.execute!( + { script: "echo 'hello from integration test'", run_in_background: true }, + { toolCallId: "test", messages: [] } + )) as BashToolResult; + + expect(spawnResult).toBeDefined(); + expect(spawnResult.success).toBe(true); + expect("backgroundProcessId" in spawnResult).toBe(true); + + // Type narrowing for background process result + if (!("backgroundProcessId" in spawnResult)) { + throw new Error("Expected background process result"); + } + const processId: string = spawnResult.backgroundProcessId; + + // Wait for output + await new Promise((resolve) => setTimeout(resolve, 200)); + + // Create bash_output tool and read output + const outputTool = createBashOutputTool(config); + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const rawOutputResult = await outputTool.execute!( + { process_id: processId }, + { toolCallId: "test2", messages: [] } + ); + + const outputResult = rawOutputResult as BashOutputToolResult; + + expect(outputResult).toBeDefined(); + + // This is the key assertion - should succeed AND have content + expect(outputResult.success).toBe(true); + if (outputResult.success) { + expect(outputResult.output).toContain("hello from integration test"); + } else { + throw new Error(`bash_output failed: ${outputResult.error}`); + } + + tempDir[Symbol.dispose](); + }); + + it("should fail to get output if using different manager instance", async () => { + // This test documents what happens if manager instances differ + // (which would be a bug in the real system) + + // Spawn with first manager + const result = await manager.spawn(runtime, testWorkspaceId, "echo 'test'", { + cwd: process.cwd(), + displayName: "test", + }); + + expect(result.success).toBe(true); + if (!result.success) return; + + // Create a DIFFERENT manager instance + const otherManager = new BackgroundProcessManager(bgOutputDir); + + // Trying to get output from different manager should fail + // because the process isn't in its internal map + const output = await otherManager.getOutput(result.processId); + expect(output.success).toBe(false); + if (!output.success) { + expect(output.error).toContain("Process not found"); + } + }); + }); + describe("exit_code file", () => { it("should write exit_code file when process exits", async () => { const result = await manager.spawn(runtime, testWorkspaceId, "exit 42", { cwd: process.cwd(), + displayName: "test", }); expect(result.success).toBe(true); diff --git a/src/node/services/backgroundProcessManager.ts b/src/node/services/backgroundProcessManager.ts index 97aea9226a..3d210b31f9 100644 --- a/src/node/services/backgroundProcessManager.ts +++ b/src/node/services/backgroundProcessManager.ts @@ -1,7 +1,9 @@ import type { Runtime, BackgroundHandle } from "@/node/runtime/Runtime"; +import { spawnProcess } from "./backgroundProcessExecutor"; import { getErrorMessage } from "@/common/utils/errors"; import { log } from "./log"; -import * as path from "path"; + +import { EventEmitter } from "events"; /** * Metadata written to meta.json for bookkeeping @@ -21,7 +23,7 @@ export interface BackgroundProcessMeta { * Represents a background process with file-based output */ export interface BackgroundProcess { - id: string; // Short unique ID (e.g., "bg-abc123") + id: string; // Process ID (display_name from the bash tool call) pid: number; // OS process ID workspaceId: string; // Owning workspace outputDir: string; // Directory containing stdout.log, stderr.log, meta.json @@ -32,22 +34,95 @@ export interface BackgroundProcess { status: "running" | "exited" | "killed" | "failed"; handle: BackgroundHandle; // For process interaction displayName?: string; // Human-readable name (e.g., "Dev Server") + /** True if this process is being waited on (foreground mode) */ + isForeground: boolean; +} + +/** + * Tracks read position for incremental output retrieval. + * Each call to getOutput() returns only new content since the last read. + */ +interface OutputReadPosition { + outputBytes: number; } /** - * Manages background bash processes for workspaces. + * Represents a foreground process that can be sent to background. + * These are processes started via runtime.exec() (not nohup) that we track + * so users can click "Background" to stop waiting for them. + */ +export interface ForegroundProcess { + /** Workspace ID */ + workspaceId: string; + /** Tool call ID that started this process (for UI to match) */ + toolCallId: string; + /** Script being executed */ + script: string; + /** Display name for the process (used as ID if sent to background) */ + displayName: string; + /** Callback to invoke when user requests backgrounding */ + onBackground: () => void; + /** Current accumulated output (for saving to files on background) */ + output: string[]; +} + +/** + * Manages bash processes for workspaces. * - * Processes are spawned via Runtime.spawnBackground() and tracked by ID. - * Output is stored in circular buffers for later retrieval. + * ALL bash commands are spawned through this manager with background-style + * infrastructure (nohup, file output, exit code trap). This enables: + * - Uniform code path for all bash commands + * - Crash resilience (output always persisted to files) + * - Seamless fgβ†’bg transition via sendToBackground() + * + * Supports incremental output retrieval via getOutput(). */ -export class BackgroundProcessManager { +/** + * Event types emitted by BackgroundProcessManager. + * The 'change' event is emitted whenever the state changes for a workspace. + */ +export interface BackgroundProcessManagerEvents { + change: [workspaceId: string]; +} + +export class BackgroundProcessManager extends EventEmitter { // NOTE: This map is in-memory only. Background processes use nohup/setsid so they // could survive app restarts, but we kill all tracked processes on shutdown via // dispose(). Rehydrating from meta.json on startup is out of scope for now. private processes = new Map(); + // Tracks read positions for incremental output retrieval + private readPositions = new Map(); + + // Base directory for process output files + private readonly bgOutputDir: string; + // Tracks foreground processes (started via runtime.exec) that can be backgrounded + // Key is toolCallId to support multiple parallel foreground processes per workspace + private foregroundProcesses = new Map(); + + constructor(bgOutputDir: string) { + super(); + this.bgOutputDir = bgOutputDir; + } + + /** Emit a change event for a workspace */ + private emitChange(workspaceId: string): void { + this.emit("change", workspaceId); + } + /** - * Spawn a new background process. + * Get the base directory for background process output files. + */ + getBgOutputDir(): string { + return this.bgOutputDir; + } + + /** + * Spawn a new process with background-style infrastructure. + * + * All processes are spawned with nohup/setsid and file-based output, + * enabling seamless fgβ†’bg transition via sendToBackground(). + * * @param runtime Runtime to spawn the process on * @param workspaceId Workspace ID for tracking/filtering * @param script Bash script to execute @@ -59,20 +134,31 @@ export class BackgroundProcessManager { script: string, config: { cwd: string; - secrets?: Record; + env?: Record; niceness?: number; - displayName?: string; + /** Human-readable name for the process - used to generate the process ID */ + displayName: string; + /** If true, process is foreground (being waited on). Default: false (background) */ + isForeground?: boolean; + /** Auto-terminate after this many seconds (background processes only) */ + timeoutSecs?: number; } ): Promise< - { success: true; processId: string; outputDir: string } | { success: false; error: string } + | { success: true; processId: string; outputDir: string; pid: number } + | { success: false; error: string } > { log.debug(`BackgroundProcessManager.spawn() called for workspace ${workspaceId}`); - // Spawn via runtime - it generates processId and creates outputDir - const result = await runtime.spawnBackground(script, { + // Process ID is the display name directly + const processId = config.displayName; + + // Spawn via executor with background infrastructure + // spawnProcess uses runtime.tempDir() internally for output directory + const result = await spawnProcess(runtime, script, { cwd: config.cwd, workspaceId, - env: config.secrets, + processId, + env: config.env, niceness: config.niceness, }); @@ -81,10 +167,7 @@ export class BackgroundProcessManager { return { success: false, error: result.error }; } - const { handle, pid } = result; - const outputDir = handle.outputDir; - // Extract processId from outputDir (last path segment) - const processId = path.basename(outputDir); + const { handle, pid, outputDir } = result; const startTime = Date.now(); // Write meta.json with process info @@ -108,13 +191,171 @@ export class BackgroundProcessManager { status: "running", handle, displayName: config.displayName, + isForeground: config.isForeground ?? false, }; // Store process in map this.processes.set(processId, proc); - log.debug(`Background process ${processId} spawned successfully with PID ${pid}`); - return { success: true, processId, outputDir }; + log.debug( + `Process ${processId} spawned successfully with PID ${pid} (foreground: ${proc.isForeground})` + ); + + // Schedule auto-termination for background processes with timeout + const timeoutSecs = config.timeoutSecs; + if (!config.isForeground && timeoutSecs !== undefined && timeoutSecs > 0) { + setTimeout(() => { + void this.terminate(processId).then((result) => { + if (result.success) { + log.debug(`Process ${processId} auto-terminated after ${timeoutSecs}s timeout`); + } + }); + }, timeoutSecs * 1000); + } + + // Emit change event (only if background - foreground processes don't show in list) + if (!proc.isForeground) { + this.emitChange(workspaceId); + } + + return { success: true, processId, outputDir, pid }; + } + + /** + * Register a foreground process that can be sent to background. + * Called by bash tool when starting foreground execution. + * + * @param workspaceId Workspace the process belongs to + * @param toolCallId Tool call ID (for UI to identify which bash row) + * @param script Script being executed + * @param onBackground Callback invoked when user requests backgrounding + * @returns Cleanup function to call when process completes + */ + registerForegroundProcess( + workspaceId: string, + toolCallId: string, + script: string, + displayName: string, + onBackground: () => void + ): { unregister: () => void; addOutput: (line: string) => void } { + const proc: ForegroundProcess = { + workspaceId, + toolCallId, + script, + displayName, + onBackground, + output: [], + }; + this.foregroundProcesses.set(toolCallId, proc); + log.debug( + `Registered foreground process for workspace ${workspaceId}, toolCallId ${toolCallId}` + ); + this.emitChange(workspaceId); + + return { + unregister: () => { + this.foregroundProcesses.delete(toolCallId); + log.debug(`Unregistered foreground process toolCallId ${toolCallId}`); + this.emitChange(workspaceId); + }, + addOutput: (line: string) => { + proc.output.push(line); + }, + }; + } + + /** + * Register a migrated foreground process as a tracked background process. + * + * Called by bash tool when migration completes, after migrateToBackground() + * has created the output directory and started file writing. + * + * @param handle The BackgroundHandle from migrateToBackground() + * @param processId The generated process ID + * @param workspaceId Workspace the process belongs to + * @param script Original script being executed + * @param outputDir Directory containing output files + * @param displayName Optional human-readable name + */ + registerMigratedProcess( + handle: BackgroundHandle, + processId: string, + workspaceId: string, + script: string, + outputDir: string, + displayName?: string + ): void { + const startTime = Date.now(); + + const proc: BackgroundProcess = { + id: processId, + pid: 0, // Unknown for migrated processes (could be remote) + workspaceId, + outputDir, + script, + startTime, + status: "running", + handle, + displayName, + isForeground: false, // Now in background + }; + + // Store process in map + this.processes.set(processId, proc); + + // Write meta.json + const meta: BackgroundProcessMeta = { + id: processId, + pid: 0, + script, + startTime, + status: "running", + displayName, + }; + void handle.writeMeta(JSON.stringify(meta, null, 2)); + + log.debug(`Migrated process ${processId} registered for workspace ${workspaceId}`); + this.emitChange(workspaceId); + } + + /** + * Send a foreground process to background. + * + * For processes started with background infrastructure (isForeground=true in spawn): + * - Marks as background and emits 'backgrounded' event + * + * For processes started via runtime.exec (tracked via registerForegroundProcess): + * - Invokes the onBackground callback to trigger early return + * + * @param toolCallId The tool call ID of the bash to background + * @returns Success status + */ + sendToBackground(toolCallId: string): { success: true } | { success: false; error: string } { + log.debug(`BackgroundProcessManager.sendToBackground(${toolCallId}) called`); + + const fgProc = this.foregroundProcesses.get(toolCallId); + if (fgProc) { + fgProc.onBackground(); + log.debug(`Foreground process toolCallId ${toolCallId} sent to background`); + return { success: true }; + } + + return { success: false, error: "No foreground process found with that tool call ID" }; + } + + /** + * Get all foreground tool call IDs for a workspace. + * Returns empty array if no foreground processes are running. + */ + getForegroundToolCallIds(workspaceId: string): string[] { + const ids: string[] = []; + // Check exec-based foreground processes + for (const [toolCallId, proc] of this.foregroundProcesses) { + if (proc.workspaceId === workspaceId) { + ids.push(toolCallId); + } + } + return ids; } /** @@ -164,14 +405,124 @@ export class BackgroundProcessManager { } /** - * List all background processes, optionally filtered by workspace. + * Get incremental output from a background process. + * Returns only NEW output since the last call (tracked per process). + * @param processId Process ID to get output from + * @param filter Optional regex pattern to filter output lines (non-matching lines are discarded permanently) + * @param timeout Seconds to wait for output if none available (0-15, default 0 = non-blocking) + */ + async getOutput( + processId: string, + filter?: string, + timeout?: number + ): Promise< + | { + success: true; + status: "running" | "exited" | "killed" | "failed"; + output: string; + exitCode?: number; + } + | { success: false; error: string } + > { + const timeoutSecs = Math.min(Math.max(timeout ?? 0, 0), 15); // Clamp to 0-15 + log.debug( + `BackgroundProcessManager.getOutput(${processId}, filter=${filter ?? "none"}, timeout=${timeoutSecs}s) called` + ); + + const proc = await this.getProcess(processId); + if (!proc) { + return { success: false, error: `Process not found: ${processId}` }; + } + + // Get or initialize read position + let pos = this.readPositions.get(processId); + if (!pos) { + pos = { outputBytes: 0 }; + this.readPositions.set(processId, pos); + } + + log.debug( + `BackgroundProcessManager.getOutput: proc.outputDir=${proc.outputDir}, offset=${pos.outputBytes}` + ); + + // Blocking wait loop: poll for output up to timeout seconds + const startTime = Date.now(); + const timeoutMs = timeoutSecs * 1000; + const pollIntervalMs = 100; + let output = ""; + let currentStatus = proc.status; + + while (true) { + // Read new content via the handle (works for both local and SSH runtimes) + // Output is already unified in output.log (stdout + stderr via 2>&1) + const result = await proc.handle.readOutput(pos.outputBytes); + output = result.content; + + // Update read position + pos.outputBytes = result.newOffset; + + // Refresh process status + const refreshedProc = await this.getProcess(processId); + currentStatus = refreshedProc?.status ?? proc.status; + + // Return immediately if: + // 1. We have output + // 2. Process is no longer running (exited/killed/failed) + // 3. Timeout elapsed + if (output.length > 0 || currentStatus !== "running") { + break; + } + + const elapsed = Date.now() - startTime; + if (elapsed >= timeoutMs) { + break; + } + + // Sleep before next poll + await new Promise((resolve) => setTimeout(resolve, pollIntervalMs)); + } + + log.debug(`BackgroundProcessManager.getOutput: read outputLen=${output.length}`); + + let filteredOutput = output; + + // Apply filter if provided (permanently discards non-matching lines) + if (filter) { + try { + const regex = new RegExp(filter); + filteredOutput = output + .split("\n") + .filter((line) => regex.test(line)) + .join("\n"); + } catch (e) { + return { success: false, error: `Invalid filter regex: ${getErrorMessage(e)}` }; + } + } + + return { + success: true, + status: currentStatus, + output: filteredOutput, + exitCode: + currentStatus !== "running" + ? ((await this.getProcess(processId))?.exitCode ?? undefined) + : undefined, + }; + } + + /** + * List background processes (not including foreground ones being waited on). + * Optionally filtered by workspace. * Refreshes status of running processes before returning. */ async list(workspaceId?: string): Promise { log.debug(`BackgroundProcessManager.list(${workspaceId ?? "all"}) called`); await this.refreshRunningStatuses(); - const allProcesses = Array.from(this.processes.values()); - return workspaceId ? allProcesses.filter((p) => p.workspaceId === workspaceId) : allProcesses; + // Only return background processes (not foreground ones being waited on) + const backgroundProcesses = Array.from(this.processes.values()).filter((p) => !p.isForeground); + return workspaceId + ? backgroundProcesses.filter((p) => p.workspaceId === workspaceId) + : backgroundProcesses; } /** @@ -236,6 +587,7 @@ export class BackgroundProcessManager { await proc.handle.dispose(); log.debug(`Process ${processId} terminated successfully`); + this.emitChange(proc.workspaceId); return { success: true }; } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); @@ -249,6 +601,7 @@ export class BackgroundProcessManager { }); // Ensure handle is cleaned up even on error await proc.handle.dispose(); + this.emitChange(proc.workspaceId); return { success: true }; } } diff --git a/src/node/services/serviceContainer.ts b/src/node/services/serviceContainer.ts index d22cd65d1f..8807c754e7 100644 --- a/src/node/services/serviceContainer.ts +++ b/src/node/services/serviceContainer.ts @@ -1,3 +1,4 @@ +import * as os from "os"; import * as path from "path"; import type { Config } from "@/node/config"; import { AIService } from "@/node/services/aiService"; @@ -63,7 +64,9 @@ export class ServiceContainer { this.extensionMetadata = new ExtensionMetadataService( path.join(config.rootDir, "extensionMetadata.json") ); - this.backgroundProcessManager = new BackgroundProcessManager(); + this.backgroundProcessManager = new BackgroundProcessManager( + path.join(os.tmpdir(), "mux-bashes") + ); this.mcpServerManager = new MCPServerManager(this.mcpConfigService); this.aiService = new AIService( config, diff --git a/src/node/services/systemMessage.test.ts b/src/node/services/systemMessage.test.ts index c26cb9fdbc..0e47c2fabb 100644 --- a/src/node/services/systemMessage.test.ts +++ b/src/node/services/systemMessage.test.ts @@ -43,7 +43,7 @@ describe("buildSystemMessage", () => { process.env.MUX_ROOT = globalDir; // Create a local runtime for tests - runtime = new LocalRuntime(tempDir, tempDir); + runtime = new LocalRuntime(tempDir); }); afterEach(async () => { diff --git a/src/node/services/tools/bash.test.ts b/src/node/services/tools/bash.test.ts index 34854c6a9f..96f6afb415 100644 --- a/src/node/services/tools/bash.test.ts +++ b/src/node/services/tools/bash.test.ts @@ -40,6 +40,7 @@ describe("bash tool", () => { script: "echo hello", timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -58,6 +59,7 @@ describe("bash tool", () => { script: "echo line1 && echo line2 && echo line3", timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -75,6 +77,7 @@ describe("bash tool", () => { run_in_background: false, script: "for i in {1..400}; do echo line$i; done", // Exceeds 300 line hard cap timeout_secs: 5, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -94,6 +97,7 @@ describe("bash tool", () => { run_in_background: false, script: "for i in {1..400}; do echo line$i; done", // Exceeds 300 line hard cap timeout_secs: 5, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -147,6 +151,7 @@ describe("bash tool", () => { run_in_background: false, script: "for i in {1..500}; do echo line$i; done", timeout_secs: 5, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -174,6 +179,7 @@ describe("bash tool", () => { script: 'perl -e \'for (1..1700) { print "A" x 900 . "\\n" }\'', timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -207,7 +213,7 @@ describe("bash tool", () => { const tool = createBashTool({ ...getTestDeps(), cwd: process.cwd(), - runtime: new LocalRuntime(process.cwd(), tempDir.path), + runtime: new LocalRuntime(process.cwd()), runtimeTempDir: tempDir.path, overflow_policy: "truncate", }); @@ -217,6 +223,7 @@ describe("bash tool", () => { script: 'perl -e \'print "A" x 2000000 . "\\n"\'', timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -243,7 +250,7 @@ describe("bash tool", () => { const tool = createBashTool({ ...getTestDeps(), cwd: process.cwd(), - runtime: new LocalRuntime(process.cwd(), tempDir.path), + runtime: new LocalRuntime(process.cwd()), runtimeTempDir: tempDir.path, overflow_policy: "truncate", }); @@ -254,6 +261,7 @@ describe("bash tool", () => { script: 'perl -e \'print "A" x 500000 . "\\n"; print "B" x 600000 . "\\n"\'', timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -283,7 +291,7 @@ describe("bash tool", () => { const tool = createBashTool({ ...getTestDeps(), cwd: process.cwd(), - runtime: new LocalRuntime(process.cwd(), tempDir.path), + runtime: new LocalRuntime(process.cwd()), runtimeTempDir: tempDir.path, // overflow_policy not specified - should default to tmpfile }); @@ -292,6 +300,7 @@ describe("bash tool", () => { run_in_background: false, script: "for i in {1..400}; do echo line$i; done", timeout_secs: 5, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -318,7 +327,7 @@ describe("bash tool", () => { const tool = createBashTool({ ...getTestDeps(), cwd: process.cwd(), - runtime: new LocalRuntime(process.cwd(), tempDir.path), + runtime: new LocalRuntime(process.cwd()), runtimeTempDir: tempDir.path, }); @@ -329,6 +338,7 @@ describe("bash tool", () => { run_in_background: false, script: "for i in {1..1300}; do printf 'line%04d with some padding text here\\n' $i; done", timeout_secs: 5, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -372,7 +382,7 @@ describe("bash tool", () => { const tool = createBashTool({ ...getTestDeps(), cwd: process.cwd(), - runtime: new LocalRuntime(process.cwd(), tempDir.path), + runtime: new LocalRuntime(process.cwd()), runtimeTempDir: tempDir.path, }); @@ -383,6 +393,7 @@ describe("bash tool", () => { run_in_background: false, script: "for i in {1..1600}; do printf 'line%04d: '; printf 'x%.0s' {1..80}; echo; done", timeout_secs: 10, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -417,7 +428,7 @@ describe("bash tool", () => { const tool = createBashTool({ ...getTestDeps(), cwd: process.cwd(), - runtime: new LocalRuntime(process.cwd(), tempDir.path), + runtime: new LocalRuntime(process.cwd()), runtimeTempDir: tempDir.path, }); @@ -428,6 +439,7 @@ describe("bash tool", () => { "for i in {1..500}; do printf 'line%04d with padding text\\n' $i; done; echo 'COMPLETION_MARKER'", timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -461,7 +473,7 @@ describe("bash tool", () => { const tool = createBashTool({ ...getTestDeps(), cwd: process.cwd(), - runtime: new LocalRuntime(process.cwd(), tempDir.path), + runtime: new LocalRuntime(process.cwd()), runtimeTempDir: tempDir.path, }); @@ -470,6 +482,7 @@ describe("bash tool", () => { run_in_background: false, script: "printf 'x%.0s' {1..2000}; echo; echo 'SHOULD_NOT_APPEAR'", timeout_secs: 5, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -503,7 +516,7 @@ describe("bash tool", () => { const tool = createBashTool({ ...getTestDeps(), cwd: process.cwd(), - runtime: new LocalRuntime(process.cwd(), tempDir.path), + runtime: new LocalRuntime(process.cwd()), runtimeTempDir: tempDir.path, }); @@ -513,6 +526,7 @@ describe("bash tool", () => { run_in_background: false, script: "for i in {1..299}; do printf 'line%04d with some padding text here now\\n' $i; done", timeout_secs: 5, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -535,7 +549,7 @@ describe("bash tool", () => { const tool = createBashTool({ ...getTestDeps(), cwd: process.cwd(), - runtime: new LocalRuntime(process.cwd(), tempDir.path), + runtime: new LocalRuntime(process.cwd()), runtimeTempDir: tempDir.path, }); @@ -544,6 +558,7 @@ describe("bash tool", () => { run_in_background: false, script: "for i in {1..300}; do printf 'line%04d\\n' $i; done", timeout_secs: 5, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -565,6 +580,7 @@ describe("bash tool", () => { script: "echo stdout1 && echo stderr1 >&2 && echo stdout2 && echo stderr2 >&2", timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -586,6 +602,7 @@ describe("bash tool", () => { script: "exit 42", timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -604,6 +621,7 @@ describe("bash tool", () => { script: "while true; do sleep 0.1; done", timeout_secs: 1, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -622,6 +640,7 @@ describe("bash tool", () => { script: "true", timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -644,6 +663,7 @@ describe("bash tool", () => { script: "echo 'test:first-child' | grep ':first-child'", timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -670,6 +690,7 @@ describe("bash tool", () => { script: "echo test | cat", timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -695,6 +716,7 @@ describe("bash tool", () => { 'python3 -c "import os,stat;mode=os.fstat(0).st_mode;print(stat.S_IFMT(mode)==stat.S_IFIFO)"', timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -740,6 +762,7 @@ describe("bash tool", () => { script: "echo test", timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -758,6 +781,7 @@ describe("bash tool", () => { script: "echo 'cd' && echo test", timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -779,6 +803,7 @@ describe("bash tool", () => { script: "while true; do sleep 1; done > /dev/null 2>&1 &", timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -800,6 +825,7 @@ describe("bash tool", () => { script: "while true; do sleep 1; done > /dev/null 2>&1 & echo $!", timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -824,6 +850,7 @@ describe("bash tool", () => { script: "while true; do sleep 0.1; done & wait", timeout_secs: 1, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -844,6 +871,7 @@ describe("bash tool", () => { script: `echo '${longLine}'`, timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -864,6 +892,7 @@ describe("bash tool", () => { script: `for i in {1..${numLines}}; do echo '${lineContent}'; done`, timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -882,6 +911,7 @@ describe("bash tool", () => { run_in_background: false, script: `for i in {1..1000}; do echo 'This is line number '$i' with some content'; done`, timeout_secs: 5, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -900,6 +930,7 @@ describe("bash tool", () => { script: "", timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -920,6 +951,7 @@ describe("bash tool", () => { script: " \n\t ", timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -939,6 +971,7 @@ describe("bash tool", () => { script: "sleep 5", timeout_secs: 10, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -960,6 +993,7 @@ describe("bash tool", () => { script: "for i in 1 2 3; do echo $i; sleep 0.1; done", timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -1039,6 +1073,7 @@ echo "$RESULT" `, timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -1067,6 +1102,7 @@ fi `, timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -1086,6 +1122,7 @@ fi script: "echo hello", timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -1109,6 +1146,7 @@ fi script: `echo "${marker}"; sleep 100 & echo $!`, timeout_secs: 1, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -1164,6 +1202,7 @@ fi `, timeout_secs: 10, run_in_background: false, + display_name: "test", }; // Start the command @@ -1234,6 +1273,7 @@ fi `, timeout_secs: 120, run_in_background: false, + display_name: "test", }; // Start the command @@ -1292,6 +1332,7 @@ describe("muxEnv environment variables", () => { const args: BashToolArgs = { script: 'echo "PROJECT:$MUX_PROJECT_PATH RUNTIME:$MUX_RUNTIME WORKSPACE:$MUX_WORKSPACE_NAME"', timeout_secs: 5, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -1320,6 +1361,7 @@ describe("muxEnv environment variables", () => { const args: BashToolArgs = { script: 'echo "MUX:$MUX_PROJECT_PATH CUSTOM:$CUSTOM_VAR"', timeout_secs: 5, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -1369,6 +1411,7 @@ describe("SSH runtime redundant cd detection", () => { script: "cd /remote/workspace/project/branch && echo test", timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -1391,6 +1434,7 @@ describe("SSH runtime redundant cd detection", () => { script: "cd /tmp && echo test", timeout_secs: 5, run_in_background: false, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -1411,6 +1455,7 @@ describe("bash tool - background execution", () => { const args: BashToolArgs = { script: "echo test", run_in_background: true, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -1421,8 +1466,8 @@ describe("bash tool - background execution", () => { } }); - it("should reject timeout with background mode", async () => { - const manager = new BackgroundProcessManager(); + it("should accept timeout with background mode for auto-termination", async () => { + const manager = new BackgroundProcessManager("/tmp/mux-test-bg"); const tempDir = new TestTempDir("test-bash-bg"); const config = createTestToolConfig(tempDir.path); @@ -1433,20 +1478,23 @@ describe("bash tool - background execution", () => { script: "echo test", timeout_secs: 5, run_in_background: true, + display_name: "test-timeout-bg", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("Cannot specify timeout with run_in_background"); + // Background with timeout should succeed - timeout is used for auto-termination + expect(result.success).toBe(true); + if (result.success && "backgroundProcessId" in result) { + expect(result.backgroundProcessId).toBe("test-timeout-bg"); } + await manager.terminateAll(); tempDir[Symbol.dispose](); }); it("should start background process and return process ID", async () => { - const manager = new BackgroundProcessManager(); + const manager = new BackgroundProcessManager("/tmp/mux-test-bg"); const tempDir = new TestTempDir("test-bash-bg"); const config = createTestToolConfig(tempDir.path); @@ -1456,6 +1504,7 @@ describe("bash tool - background execution", () => { const args: BashToolArgs = { script: "echo hello", run_in_background: true, + display_name: "test", }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; @@ -1463,7 +1512,8 @@ describe("bash tool - background execution", () => { expect(result.success).toBe(true); if (result.success && "backgroundProcessId" in result) { expect(result.backgroundProcessId).toBeDefined(); - expect(result.backgroundProcessId).toMatch(/^bg-/); + // Process ID is now the display name directly + expect(result.backgroundProcessId).toBe("test"); } else { throw new Error("Expected background process ID in result"); } diff --git a/src/node/services/tools/bash.ts b/src/node/services/tools/bash.ts index d6f5426ef8..6b7038d1c6 100644 --- a/src/node/services/tools/bash.ts +++ b/src/node/services/tools/bash.ts @@ -15,6 +15,7 @@ import { EXIT_CODE_ABORTED, EXIT_CODE_TIMEOUT } from "@/common/constants/exitCod import type { BashToolResult } from "@/common/types/tools"; import type { ToolConfiguration, ToolFactory } from "@/common/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/common/utils/tools/toolDefinitions"; +import { migrateToBackground } from "@/node/services/backgroundProcessExecutor"; /** * Validates bash script input for common issues @@ -231,13 +232,13 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { inputSchema: TOOL_DEFINITIONS.bash.schema, execute: async ( { script, timeout_secs, run_in_background, display_name }, - { abortSignal } + { abortSignal, toolCallId } ): Promise => { // Validate script input const validationError = validateScript(script, config); if (validationError) return validationError; - // Handle background execution + // Handle explicit background execution (run_in_background=true) if (run_in_background) { if (!config.workspaceId || !config.backgroundProcessManager || !config.runtime) { return { @@ -249,15 +250,6 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { }; } - if (timeout_secs !== undefined) { - return { - success: false, - error: "Cannot specify timeout with run_in_background", - exitCode: -1, - wall_duration_ms: 0, - }; - } - const startTime = performance.now(); const spawnResult = await config.backgroundProcessManager.spawn( config.runtime, @@ -265,9 +257,11 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { script, { cwd: config.cwd, - secrets: config.secrets, + env: config.secrets, niceness: config.niceness, displayName: display_name, + isForeground: false, // Explicit background + timeoutSecs: timeout_secs, // Auto-terminate after this duration } ); @@ -280,17 +274,12 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { }; } - const stdoutPath = `${spawnResult.outputDir}/stdout.log`; - const stderrPath = `${spawnResult.outputDir}/stderr.log`; - return { success: true, output: `Background process started with ID: ${spawnResult.processId}`, exitCode: 0, wall_duration_ms: Math.round(performance.now() - startTime), backgroundProcessId: spawnResult.processId, - stdout_path: stdoutPath, - stderr_path: stderrPath, }; } @@ -301,6 +290,39 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { let overflowReason: string | null = null; const truncationState = { displayTruncated: false, fileTruncated: false }; + // Track backgrounding state + let backgrounded = false; + let backgroundResolve: (() => void) | null = null; + + // Wrap abort signal so we can detach when migrating to background. + // When detached, the original stream abort won't kill the process. + const wrappedAbortController = new AbortController(); + let abortDetached = false; + if (abortSignal) { + abortSignal.addEventListener("abort", () => { + if (!abortDetached) { + wrappedAbortController.abort(); + } + }); + } + + // Register foreground process for "send to background" feature + // Only if manager is available (AI tool calls, not IPC) + const fgRegistration = + config.backgroundProcessManager && config.workspaceId && toolCallId + ? config.backgroundProcessManager.registerForegroundProcess( + config.workspaceId, + toolCallId, + script, + display_name, + () => { + backgrounded = true; + // Resolve the background promise to unblock the wait + if (backgroundResolve) backgroundResolve(); + } + ) + : null; + // Execute using runtime interface (works for both local and SSH) const scriptWithClosedStdin = `exec { + // Cancel all stream branches to stop the process and unblock readers + stdoutForUI.cancel().catch(() => { /* ignore */ return; }); - execStream.stderr.cancel().catch(() => { + stderrForUI.cancel().catch(() => { + /* ignore */ return; + }); + stdoutForMigration.cancel().catch(() => { + /* ignore */ return; + }); + stderrForMigration.cancel().catch(() => { /* ignore */ return; }); }; @@ -425,15 +458,97 @@ ${script}`; } }; - // Start consuming stdout and stderr concurrently - const consumeStdout = consumeStream(execStream.stdout); - const consumeStderr = consumeStream(execStream.stderr); + // Start consuming stdout and stderr concurrently (using UI branches) + const consumeStdout = consumeStream(stdoutForUI); + const consumeStderr = consumeStream(stderrForUI); + + // Create a promise that resolves when user clicks "Background" + const backgroundPromise = new Promise((resolve) => { + backgroundResolve = resolve; + }); // Wait for process exit and stream consumption concurrently + // Also race with the background promise to detect early return request let exitCode: number; try { - [exitCode] = await Promise.all([execStream.exitCode, consumeStdout, consumeStderr]); + const result = await Promise.race([ + Promise.all([execStream.exitCode, consumeStdout, consumeStderr]), + backgroundPromise.then(() => "backgrounded" as const), + ]); + + // Check if we were backgrounded + if (result === "backgrounded" || backgrounded) { + // Unregister foreground process + fgRegistration?.unregister(); + + // Detach from abort signal - process should continue running + // even when the stream ends and fires abort + abortDetached = true; + + const wall_duration_ms = Math.round(performance.now() - startTime); + + // Migrate to background tracking if manager is available + if (config.backgroundProcessManager && config.workspaceId) { + const processId = display_name; + + // Create a synthetic ExecStream for the migration streams + // The UI streams are still being consumed, migration streams continue to files + const migrationStream = { + stdout: stdoutForMigration, + stderr: stderrForMigration, + stdin: execStream.stdin, + exitCode: execStream.exitCode, + duration: execStream.duration, + }; + + const migrateResult = await migrateToBackground( + migrationStream, + { + cwd: config.cwd, + workspaceId: config.workspaceId, + processId, + script, + existingOutput: lines, + }, + config.backgroundProcessManager.getBgOutputDir() + ); + + if (migrateResult.success) { + // Register the migrated process with the manager + config.backgroundProcessManager.registerMigratedProcess( + migrateResult.handle, + processId, + config.workspaceId, + script, + migrateResult.outputDir + ); + + return { + success: true, + output: `Process sent to background with ID: ${processId}\n\nOutput so far (${lines.length} lines):\n${lines.slice(-20).join("\n")}${lines.length > 20 ? "\n...(showing last 20 lines)" : ""}`, + exitCode: 0, + wall_duration_ms, + backgroundProcessId: processId, + }; + } + // Migration failed, fall through to simple return + } + + // Fallback: return without process ID (no manager or migration failed) + return { + success: true, + output: `Process sent to background. It will continue running.\n\nOutput so far (${lines.length} lines):\n${lines.slice(-20).join("\n")}${lines.length > 20 ? "\n...(showing last 20 lines)" : ""}`, + exitCode: 0, + wall_duration_ms, + }; + } + + // Normal completion - extract exit code + exitCode = result[0]; } catch (err: unknown) { + // Unregister on error + fgRegistration?.unregister(); + // Check if this was an abort if (abortSignal?.aborted) { return { @@ -451,6 +566,9 @@ ${script}`; }; } + // Unregister foreground process on normal completion + fgRegistration?.unregister(); + // Check if command was aborted (exitCode will be EXIT_CODE_ABORTED = -997) // This can happen if abort signal fired after Promise.all resolved but before we check if (abortSignal?.aborted) { diff --git a/src/node/services/tools/bash_background_list.test.ts b/src/node/services/tools/bash_background_list.test.ts index 77137e43b5..ee528d72a6 100644 --- a/src/node/services/tools/bash_background_list.test.ts +++ b/src/node/services/tools/bash_background_list.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from "bun:test"; +import { describe, it, expect, afterEach } from "bun:test"; import { createBashBackgroundListTool } from "./bash_background_list"; import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; import { LocalRuntime } from "@/node/runtime/LocalRuntime"; @@ -6,18 +6,29 @@ import type { Runtime } from "@/node/runtime/Runtime"; import type { BashBackgroundListResult } from "@/common/types/tools"; import { TestTempDir, createTestToolConfig } from "./testHelpers"; import type { ToolCallOptions } from "ai"; +import * as fs from "fs/promises"; const mockToolCallOptions: ToolCallOptions = { toolCallId: "test-call-id", messages: [], }; -// Create test runtime with isolated sessions directory -function createTestRuntime(sessionsDir: string): Runtime { - return new LocalRuntime(process.cwd(), sessionsDir); +// Create test runtime +function createTestRuntime(): Runtime { + return new LocalRuntime(process.cwd()); } +// Workspace IDs used in tests - need cleanup after each test +const TEST_WORKSPACES = ["test-workspace", "workspace-a", "workspace-b"]; + describe("bash_background_list tool", () => { + afterEach(async () => { + // Clean up output directories from /tmp/mux-bashes/ to prevent test pollution + for (const ws of TEST_WORKSPACES) { + await fs.rm(`/tmp/mux-bashes/${ws}`, { recursive: true, force: true }).catch(() => undefined); + } + }); + it("should return error when manager not available", async () => { const tempDir = new TestTempDir("test-bash-bg-list"); const config = createTestToolConfig(process.cwd()); @@ -35,8 +46,8 @@ describe("bash_background_list tool", () => { }); it("should return error when workspaceId not available", async () => { - const manager = new BackgroundProcessManager(); const tempDir = new TestTempDir("test-bash-bg-list"); + const manager = new BackgroundProcessManager(tempDir.path); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; @@ -54,8 +65,8 @@ describe("bash_background_list tool", () => { }); it("should return empty list when no processes", async () => { - const manager = new BackgroundProcessManager(); const tempDir = new TestTempDir("test-bash-bg-list"); + const manager = new BackgroundProcessManager(tempDir.path); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; @@ -72,9 +83,9 @@ describe("bash_background_list tool", () => { }); it("should list spawned processes with correct fields", async () => { - const manager = new BackgroundProcessManager(); const tempDir = new TestTempDir("test-bash-bg-list"); - const runtime = createTestRuntime(tempDir.path); + const manager = new BackgroundProcessManager(tempDir.path); + const runtime = createTestRuntime(); const config = createTestToolConfig(process.cwd(), { sessionsDir: tempDir.path }); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; @@ -82,6 +93,7 @@ describe("bash_background_list tool", () => { // Spawn a process const spawnResult = await manager.spawn(runtime, "test-workspace", "sleep 10", { cwd: process.cwd(), + displayName: "test", }); if (!spawnResult.success) { @@ -100,8 +112,6 @@ describe("bash_background_list tool", () => { expect(proc.script).toBe("sleep 10"); expect(proc.uptime_ms).toBeGreaterThanOrEqual(0); expect(proc.exitCode).toBeUndefined(); - expect(proc.stdout_path).toContain("stdout.log"); - expect(proc.stderr_path).toContain("stderr.log"); } // Cleanup @@ -110,9 +120,9 @@ describe("bash_background_list tool", () => { }); it("should include display_name in listed processes", async () => { - const manager = new BackgroundProcessManager(); const tempDir = new TestTempDir("test-bash-bg-list"); - const runtime = createTestRuntime(tempDir.path); + const manager = new BackgroundProcessManager(tempDir.path); + const runtime = createTestRuntime(); const config = createTestToolConfig(process.cwd(), { sessionsDir: tempDir.path }); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; @@ -124,7 +134,7 @@ describe("bash_background_list tool", () => { }); if (!spawnResult.success) { - throw new Error("Failed to spawn process"); + throw new Error(`Failed to spawn process: ${spawnResult.error}`); } const tool = createBashBackgroundListTool(config); @@ -142,9 +152,9 @@ describe("bash_background_list tool", () => { }); it("should only list processes for the current workspace", async () => { - const manager = new BackgroundProcessManager(); const tempDir = new TestTempDir("test-bash-bg-list"); - const runtime = createTestRuntime(tempDir.path); + const manager = new BackgroundProcessManager(tempDir.path); + const runtime = createTestRuntime(); const config = createTestToolConfig(process.cwd(), { workspaceId: "workspace-a", @@ -156,9 +166,11 @@ describe("bash_background_list tool", () => { // Spawn processes in different workspaces const spawnA = await manager.spawn(runtime, "workspace-a", "sleep 10", { cwd: process.cwd(), + displayName: "test-a", }); const spawnB = await manager.spawn(runtime, "workspace-b", "sleep 10", { cwd: process.cwd(), + displayName: "test-b", }); if (!spawnA.success || !spawnB.success) { diff --git a/src/node/services/tools/bash_background_list.ts b/src/node/services/tools/bash_background_list.ts index 7e7289fe7e..ae7732eb26 100644 --- a/src/node/services/tools/bash_background_list.ts +++ b/src/node/services/tools/bash_background_list.ts @@ -36,8 +36,6 @@ export const createBashBackgroundListTool: ToolFactory = (config: ToolConfigurat script: p.script, uptime_ms: p.exitTime !== undefined ? p.exitTime - p.startTime : now - p.startTime, exitCode: p.exitCode, - stdout_path: `${p.outputDir}/stdout.log`, - stderr_path: `${p.outputDir}/stderr.log`, display_name: p.displayName, })), }; diff --git a/src/node/services/tools/bash_background_terminate.test.ts b/src/node/services/tools/bash_background_terminate.test.ts index 4687d0235a..b5c78e9577 100644 --- a/src/node/services/tools/bash_background_terminate.test.ts +++ b/src/node/services/tools/bash_background_terminate.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from "bun:test"; +import { describe, it, expect, afterEach } from "bun:test"; import { createBashBackgroundTerminateTool } from "./bash_background_terminate"; import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; import { LocalRuntime } from "@/node/runtime/LocalRuntime"; @@ -9,18 +9,28 @@ import type { } from "@/common/types/tools"; import { TestTempDir, createTestToolConfig } from "./testHelpers"; import type { ToolCallOptions } from "ai"; +import * as fs from "fs/promises"; const mockToolCallOptions: ToolCallOptions = { toolCallId: "test-call-id", messages: [], }; -// Create test runtime with isolated sessions directory -function createTestRuntime(sessionsDir: string): Runtime { - return new LocalRuntime(process.cwd(), sessionsDir); +// Create test runtime +function createTestRuntime(): Runtime { + return new LocalRuntime(process.cwd()); } +// Workspace IDs used in tests - need cleanup after each test +const TEST_WORKSPACES = ["test-workspace", "workspace-a", "workspace-b"]; + describe("bash_background_terminate tool", () => { + afterEach(async () => { + // Clean up output directories from /tmp/mux-bashes/ to prevent test pollution + for (const ws of TEST_WORKSPACES) { + await fs.rm(`/tmp/mux-bashes/${ws}`, { recursive: true, force: true }).catch(() => undefined); + } + }); it("should return error when manager not available", async () => { const tempDir = new TestTempDir("test-bash-bg-term"); const config = createTestToolConfig(process.cwd()); @@ -45,8 +55,8 @@ describe("bash_background_terminate tool", () => { }); it("should return error for non-existent process", async () => { - const manager = new BackgroundProcessManager(); const tempDir = new TestTempDir("test-bash-bg-term"); + const manager = new BackgroundProcessManager(tempDir.path); const config = createTestToolConfig(process.cwd()); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; @@ -65,9 +75,9 @@ describe("bash_background_terminate tool", () => { }); it("should terminate a running process", async () => { - const manager = new BackgroundProcessManager(); const tempDir = new TestTempDir("test-bash-bg-term"); - const runtime = createTestRuntime(tempDir.path); + const manager = new BackgroundProcessManager(tempDir.path); + const runtime = createTestRuntime(); const config = createTestToolConfig(process.cwd(), { sessionsDir: tempDir.path }); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; @@ -75,6 +85,7 @@ describe("bash_background_terminate tool", () => { // Spawn a long-running process const spawnResult = await manager.spawn(runtime, "test-workspace", "sleep 10", { cwd: process.cwd(), + displayName: "test", }); if (!spawnResult.success) { @@ -105,9 +116,9 @@ describe("bash_background_terminate tool", () => { }); it("should be idempotent (double-terminate succeeds)", async () => { - const manager = new BackgroundProcessManager(); const tempDir = new TestTempDir("test-bash-bg-term"); - const runtime = createTestRuntime(tempDir.path); + const manager = new BackgroundProcessManager(tempDir.path); + const runtime = createTestRuntime(); const config = createTestToolConfig(process.cwd(), { sessionsDir: tempDir.path }); config.runtimeTempDir = tempDir.path; config.backgroundProcessManager = manager; @@ -115,6 +126,7 @@ describe("bash_background_terminate tool", () => { // Spawn a process const spawnResult = await manager.spawn(runtime, "test-workspace", "sleep 10", { cwd: process.cwd(), + displayName: "test", }); if (!spawnResult.success) { @@ -145,9 +157,9 @@ describe("bash_background_terminate tool", () => { }); it("should not terminate processes from other workspaces", async () => { - const manager = new BackgroundProcessManager(); const tempDir = new TestTempDir("test-bash-bg-term"); - const runtime = createTestRuntime(tempDir.path); + const manager = new BackgroundProcessManager(tempDir.path); + const runtime = createTestRuntime(); // Config is for workspace-a const config = createTestToolConfig(process.cwd(), { @@ -160,6 +172,7 @@ describe("bash_background_terminate tool", () => { // Spawn process in workspace-b const spawnResult = await manager.spawn(runtime, "workspace-b", "sleep 10", { cwd: process.cwd(), + displayName: "test", }); if (!spawnResult.success) { diff --git a/src/node/services/tools/bash_output.test.ts b/src/node/services/tools/bash_output.test.ts new file mode 100644 index 0000000000..0d3efe423f --- /dev/null +++ b/src/node/services/tools/bash_output.test.ts @@ -0,0 +1,484 @@ +import { describe, it, expect } from "bun:test"; +import { createBashOutputTool } from "./bash_output"; +import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; +import { LocalRuntime } from "@/node/runtime/LocalRuntime"; +import type { Runtime } from "@/node/runtime/Runtime"; +import type { BashOutputToolResult } from "@/common/types/tools"; +import { TestTempDir, createTestToolConfig } from "./testHelpers"; +import type { ToolCallOptions } from "ai"; + +const mockToolCallOptions: ToolCallOptions = { + toolCallId: "test-call-id", + messages: [], +}; + +// Create test runtime +function createTestRuntime(): Runtime { + return new LocalRuntime(process.cwd()); +} + +describe("bash_output tool", () => { + it("should return error when manager not available", async () => { + const tempDir = new TestTempDir("test-bash-output"); + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + + const tool = createBashOutputTool(config); + const result = (await tool.execute!( + { process_id: "bash_1", timeout_secs: 0 }, + mockToolCallOptions + )) as BashOutputToolResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Background process manager not available"); + } + + tempDir[Symbol.dispose](); + }); + + it("should return error when workspaceId not available", async () => { + const tempDir = new TestTempDir("test-bash-output"); + const manager = new BackgroundProcessManager(tempDir.path); + + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + delete config.workspaceId; + + const tool = createBashOutputTool(config); + const result = (await tool.execute!( + { process_id: "bash_1", timeout_secs: 0 }, + mockToolCallOptions + )) as BashOutputToolResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Workspace ID not available"); + } + + tempDir[Symbol.dispose](); + }); + + it("should return error for non-existent process", async () => { + const tempDir = new TestTempDir("test-bash-output"); + const manager = new BackgroundProcessManager(tempDir.path); + + const config = createTestToolConfig(process.cwd()); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + const tool = createBashOutputTool(config); + const result = (await tool.execute!( + { process_id: "bash_1", timeout_secs: 0 }, + mockToolCallOptions + )) as BashOutputToolResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Process not found"); + } + + tempDir[Symbol.dispose](); + }); + + it("should return incremental output from process", async () => { + const tempDir = new TestTempDir("test-bash-output"); + const manager = new BackgroundProcessManager(tempDir.path); + + const runtime = createTestRuntime(); + const config = createTestToolConfig(process.cwd(), { sessionsDir: tempDir.path }); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + // Spawn a process that outputs incrementally + const spawnResult = await manager.spawn( + runtime, + "test-workspace", + "echo 'line1'; sleep 0.5; echo 'line2'", + { cwd: process.cwd(), displayName: "test" } + ); + + if (!spawnResult.success) { + throw new Error("Failed to spawn process"); + } + + const tool = createBashOutputTool(config); + + // Wait a bit for first output + await new Promise((r) => setTimeout(r, 200)); + + // First call - should get some output + const result1 = (await tool.execute!( + { process_id: spawnResult.processId, timeout_secs: 0 }, + mockToolCallOptions + )) as BashOutputToolResult; + + expect(result1.success).toBe(true); + if (result1.success) { + expect(result1.output).toContain("line1"); + } + + // Wait for more output + await new Promise((r) => setTimeout(r, 600)); + + // Second call - should ONLY get new output (incremental) + const result2 = (await tool.execute!( + { process_id: spawnResult.processId, timeout_secs: 0 }, + mockToolCallOptions + )) as BashOutputToolResult; + + expect(result2.success).toBe(true); + if (result2.success) { + // Should contain line2 but NOT line1 (already read) + expect(result2.output).toContain("line2"); + expect(result2.output).not.toContain("line1"); + } + + // Cleanup + await manager.cleanup("test-workspace"); + tempDir[Symbol.dispose](); + }); + + it("should filter output with regex", async () => { + const tempDir = new TestTempDir("test-bash-output"); + const manager = new BackgroundProcessManager(tempDir.path); + + const runtime = createTestRuntime(); + const config = createTestToolConfig(process.cwd(), { sessionsDir: tempDir.path }); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + // Spawn a process that outputs multiple lines + const spawnResult = await manager.spawn( + runtime, + "test-workspace", + "echo 'ERROR: something failed'; echo 'INFO: everything ok'; echo 'ERROR: another error'", + { cwd: process.cwd(), displayName: "test" } + ); + + if (!spawnResult.success) { + throw new Error("Failed to spawn process"); + } + + // Wait for output + await new Promise((r) => setTimeout(r, 200)); + + const tool = createBashOutputTool(config); + const result = (await tool.execute!( + { process_id: spawnResult.processId, filter: "ERROR", timeout_secs: 0 }, + mockToolCallOptions + )) as BashOutputToolResult; + + expect(result.success).toBe(true); + if (result.success) { + // Should only contain ERROR lines + expect(result.output).toContain("ERROR"); + expect(result.output).not.toContain("INFO"); + } + + // Cleanup + await manager.cleanup("test-workspace"); + tempDir[Symbol.dispose](); + }); + + it("should return error for invalid regex filter", async () => { + const tempDir = new TestTempDir("test-bash-output"); + const manager = new BackgroundProcessManager(tempDir.path); + + const runtime = createTestRuntime(); + const config = createTestToolConfig(process.cwd(), { sessionsDir: tempDir.path }); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + const spawnResult = await manager.spawn(runtime, "test-workspace", "echo 'test'", { + cwd: process.cwd(), + displayName: "test", + }); + + if (!spawnResult.success) { + throw new Error("Failed to spawn process"); + } + + // Wait for output + await new Promise((r) => setTimeout(r, 100)); + + const tool = createBashOutputTool(config); + const result = (await tool.execute!( + { process_id: spawnResult.processId, filter: "[invalid(", timeout_secs: 0 }, + mockToolCallOptions + )) as BashOutputToolResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Invalid filter regex"); + } + + // Cleanup + await manager.cleanup("test-workspace"); + tempDir[Symbol.dispose](); + }); + + it("should not return output from other workspace's processes", async () => { + const tempDir = new TestTempDir("test-bash-output"); + const manager = new BackgroundProcessManager(tempDir.path); + + const runtime = createTestRuntime(); + + const config = createTestToolConfig(process.cwd(), { + workspaceId: "workspace-a", + sessionsDir: tempDir.path, + }); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + // Spawn process in different workspace + const spawnResult = await manager.spawn(runtime, "workspace-b", "echo 'test'", { + cwd: process.cwd(), + displayName: "test", + }); + + if (!spawnResult.success) { + throw new Error("Failed to spawn process"); + } + + const tool = createBashOutputTool(config); + const result = (await tool.execute!( + { process_id: spawnResult.processId, timeout_secs: 0 }, + mockToolCallOptions + )) as BashOutputToolResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Process not found"); + } + + // Cleanup + await manager.cleanup("workspace-b"); + tempDir[Symbol.dispose](); + }); + + it("should include process status and exit code", async () => { + const tempDir = new TestTempDir("test-bash-output"); + const manager = new BackgroundProcessManager(tempDir.path); + + const runtime = createTestRuntime(); + const config = createTestToolConfig(process.cwd(), { sessionsDir: tempDir.path }); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + // Spawn a process that exits quickly + const spawnResult = await manager.spawn(runtime, "test-workspace", "echo 'done'", { + cwd: process.cwd(), + displayName: "test", + }); + + if (!spawnResult.success) { + throw new Error("Failed to spawn process"); + } + + // Wait for process to exit + await new Promise((r) => setTimeout(r, 200)); + + const tool = createBashOutputTool(config); + const result = (await tool.execute!( + { process_id: spawnResult.processId, timeout_secs: 0 }, + mockToolCallOptions + )) as BashOutputToolResult; + + expect(result.success).toBe(true); + if (result.success) { + expect(result.status).toBe("exited"); + expect(result.exitCode).toBe(0); + } + + // Cleanup + await manager.cleanup("test-workspace"); + tempDir[Symbol.dispose](); + }); + + it("should block and wait for output when timeout_secs > 0", async () => { + const tempDir = new TestTempDir("test-bash-output"); + const manager = new BackgroundProcessManager(tempDir.path); + + const runtime = createTestRuntime(); + const config = createTestToolConfig(process.cwd(), { sessionsDir: tempDir.path }); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + // Use unique process ID to avoid leftover state from previous runs + const processId = `delayed-output-${Date.now()}`; + + // Spawn a process that outputs after a delay (use longer sleep to avoid race) + const spawnResult = await manager.spawn( + runtime, + "test-workspace", + "sleep 1; echo 'delayed output'", + { cwd: process.cwd(), displayName: processId } + ); + + if (!spawnResult.success) { + throw new Error("Failed to spawn process"); + } + + const tool = createBashOutputTool(config); + + // Call with timeout=3 should wait and return output (waiting for the sleep to complete) + const start = Date.now(); + const result = (await tool.execute!( + { process_id: spawnResult.processId, timeout_secs: 3 }, + mockToolCallOptions + )) as BashOutputToolResult; + const elapsed = Date.now() - start; + + expect(result.success).toBe(true); + if (result.success) { + expect(result.output).toContain("delayed output"); + // Should have waited at least ~1 second for the sleep + expect(elapsed).toBeGreaterThan(800); + // But not the full 3s timeout + expect(elapsed).toBeLessThan(2500); + } + + // Cleanup + await manager.cleanup("test-workspace"); + tempDir[Symbol.dispose](); + }); + + it("should return early when process exits during wait", async () => { + const tempDir = new TestTempDir("test-bash-output"); + const manager = new BackgroundProcessManager(tempDir.path); + + const runtime = createTestRuntime(); + const config = createTestToolConfig(process.cwd(), { sessionsDir: tempDir.path }); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + // Use unique process ID to avoid leftover state from previous runs + const processId = `quick-exit-${Date.now()}`; + + // Spawn a process that exits quickly + const spawnResult = await manager.spawn(runtime, "test-workspace", "echo 'quick exit'", { + cwd: process.cwd(), + displayName: processId, + }); + + if (!spawnResult.success) { + throw new Error("Failed to spawn process"); + } + + // Wait for process to exit + await new Promise((r) => setTimeout(r, 200)); + + const tool = createBashOutputTool(config); + + // Call with long timeout - should return quickly since process already exited + const start = Date.now(); + const result = (await tool.execute!( + { process_id: spawnResult.processId, timeout_secs: 10 }, + mockToolCallOptions + )) as BashOutputToolResult; + const elapsed = Date.now() - start; + + expect(result.success).toBe(true); + if (result.success) { + expect(result.output).toContain("quick exit"); + expect(result.status).toBe("exited"); + expect(elapsed).toBeLessThan(500); // Should return quickly, not wait full 10s + } + + // Cleanup + await manager.cleanup("test-workspace"); + tempDir[Symbol.dispose](); + }); + + it("should wait full timeout duration when no output and process running", async () => { + const tempDir = new TestTempDir("test-bash-output"); + const manager = new BackgroundProcessManager(tempDir.path); + + const runtime = createTestRuntime(); + const config = createTestToolConfig(process.cwd(), { sessionsDir: tempDir.path }); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + const processId = `long-sleep-${Date.now()}`; + + // Spawn a process that sleeps for a long time with no output + const spawnResult = await manager.spawn(runtime, "test-workspace", "sleep 30", { + cwd: process.cwd(), + displayName: processId, + }); + + if (!spawnResult.success) { + throw new Error("Failed to spawn process"); + } + + const tool = createBashOutputTool(config); + + // Call with short timeout - should wait full duration then return empty + const start = Date.now(); + const result = (await tool.execute!( + { process_id: spawnResult.processId, timeout_secs: 1 }, + mockToolCallOptions + )) as BashOutputToolResult; + const elapsed = Date.now() - start; + + expect(result.success).toBe(true); + if (result.success) { + expect(result.output).toBe(""); // No output + expect(result.status).toBe("running"); // Process still running + // Should have waited close to 1 second + expect(elapsed).toBeGreaterThan(900); + expect(elapsed).toBeLessThan(1500); + } + + // Cleanup + await manager.terminate(spawnResult.processId); + await manager.cleanup("test-workspace"); + tempDir[Symbol.dispose](); + }); + + it("should return immediately with timeout_secs: 0 even when no output", async () => { + const tempDir = new TestTempDir("test-bash-output"); + const manager = new BackgroundProcessManager(tempDir.path); + + const runtime = createTestRuntime(); + const config = createTestToolConfig(process.cwd(), { sessionsDir: tempDir.path }); + config.runtimeTempDir = tempDir.path; + config.backgroundProcessManager = manager; + + const processId = `no-wait-${Date.now()}`; + + // Spawn a process that sleeps (no immediate output) + const spawnResult = await manager.spawn(runtime, "test-workspace", "sleep 30", { + cwd: process.cwd(), + displayName: processId, + }); + + if (!spawnResult.success) { + throw new Error("Failed to spawn process"); + } + + const tool = createBashOutputTool(config); + + // Call with timeout=0 - should return immediately + const start = Date.now(); + const result = (await tool.execute!( + { process_id: spawnResult.processId, timeout_secs: 0 }, + mockToolCallOptions + )) as BashOutputToolResult; + const elapsed = Date.now() - start; + + expect(result.success).toBe(true); + if (result.success) { + expect(result.output).toBe(""); + expect(result.status).toBe("running"); + expect(elapsed).toBeLessThan(200); // Should return almost immediately + } + + // Cleanup + await manager.terminate(spawnResult.processId); + await manager.cleanup("test-workspace"); + tempDir[Symbol.dispose](); + }); +}); diff --git a/src/node/services/tools/bash_output.ts b/src/node/services/tools/bash_output.ts new file mode 100644 index 0000000000..c7b18027b8 --- /dev/null +++ b/src/node/services/tools/bash_output.ts @@ -0,0 +1,42 @@ +import { tool } from "ai"; +import type { BashOutputToolResult } from "@/common/types/tools"; +import type { ToolConfiguration, ToolFactory } from "@/common/utils/tools/tools"; +import { TOOL_DEFINITIONS } from "@/common/utils/tools/toolDefinitions"; + +/** + * Tool for retrieving incremental output from background processes. + * Mimics Claude Code's BashOutput tool - returns only new output since last check. + */ +export const createBashOutputTool: ToolFactory = (config: ToolConfiguration) => { + return tool({ + description: TOOL_DEFINITIONS.bash_output.description, + inputSchema: TOOL_DEFINITIONS.bash_output.schema, + execute: async ({ process_id, filter, timeout_secs }): Promise => { + if (!config.backgroundProcessManager) { + return { + success: false, + error: "Background process manager not available", + }; + } + + if (!config.workspaceId) { + return { + success: false, + error: "Workspace ID not available", + }; + } + + // Verify process belongs to this workspace + const proc = await config.backgroundProcessManager.getProcess(process_id); + if (!proc || proc.workspaceId !== config.workspaceId) { + return { + success: false, + error: `Process not found: ${process_id}`, + }; + } + + // Get incremental output with blocking wait + return await config.backgroundProcessManager.getOutput(process_id, filter, timeout_secs); + }, + }); +}; diff --git a/src/node/services/tools/file_edit_operation.test.ts b/src/node/services/tools/file_edit_operation.test.ts index fe480f2e75..7c24182742 100644 --- a/src/node/services/tools/file_edit_operation.test.ts +++ b/src/node/services/tools/file_edit_operation.test.ts @@ -154,7 +154,7 @@ describe("executeFileEditOperation plan mode enforcement", () => { const result = await executeFileEditOperation({ config: { cwd: workspaceCwd, - runtime: new LocalRuntime(workspaceCwd, tempDir.path), + runtime: new LocalRuntime(workspaceCwd), runtimeTempDir: tempDir.path, mode: "plan", planFilePath: planPath, @@ -176,7 +176,7 @@ describe("executeFileEditOperation plan mode enforcement", () => { const result = await executeFileEditOperation({ config: { cwd: tempDir.path, - runtime: new LocalRuntime(tempDir.path, tempDir.path), + runtime: new LocalRuntime(tempDir.path), runtimeTempDir: tempDir.path, mode: "exec", // No planFilePath in exec mode @@ -198,7 +198,7 @@ describe("executeFileEditOperation plan mode enforcement", () => { const result = await executeFileEditOperation({ config: { cwd: tempDir.path, - runtime: new LocalRuntime(tempDir.path, tempDir.path), + runtime: new LocalRuntime(tempDir.path), runtimeTempDir: tempDir.path, // mode is undefined }, diff --git a/src/node/services/tools/file_read.test.ts b/src/node/services/tools/file_read.test.ts index edd021f36f..b14cb74aae 100644 --- a/src/node/services/tools/file_read.test.ts +++ b/src/node/services/tools/file_read.test.ts @@ -354,7 +354,7 @@ describe("file_read tool", () => { const tool = createFileReadTool({ ...getTestDeps(), cwd: subDir, - runtime: new LocalRuntime(process.cwd(), testDir), + runtime: new LocalRuntime(process.cwd()), runtimeTempDir: testDir, }); const args: FileReadToolArgs = { diff --git a/src/node/services/tools/testHelpers.ts b/src/node/services/tools/testHelpers.ts index 5274957b7a..b5b89e97a9 100644 --- a/src/node/services/tools/testHelpers.ts +++ b/src/node/services/tools/testHelpers.ts @@ -56,7 +56,7 @@ export function createTestToolConfig( ): ToolConfiguration { return { cwd: tempDir, - runtime: new LocalRuntime(tempDir, options?.sessionsDir ?? tempDir), + runtime: new LocalRuntime(tempDir), runtimeTempDir: tempDir, niceness: options?.niceness, workspaceId: options?.workspaceId ?? "test-workspace", diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index f9520dce42..0e5c4f8e9b 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -1019,4 +1019,86 @@ export class WorkspaceService extends EventEmitter { return Err(`Failed to execute bash command: ${message}`); } } + + /** + * List background processes for a workspace. + * Returns process info suitable for UI display (excludes handle). + */ + async listBackgroundProcesses(workspaceId: string): Promise< + Array<{ + id: string; + pid: number; + script: string; + displayName?: string; + startTime: number; + status: "running" | "exited" | "killed" | "failed"; + exitCode?: number; + }> + > { + const processes = await this.backgroundProcessManager.list(workspaceId); + return processes.map((p) => ({ + id: p.id, + pid: p.pid, + script: p.script, + displayName: p.displayName, + startTime: p.startTime, + status: p.status, + exitCode: p.exitCode, + })); + } + + /** + * Terminate a background process by ID. + * Verifies the process belongs to the specified workspace. + */ + async terminateBackgroundProcess(workspaceId: string, processId: string): Promise> { + // Get process to verify workspace ownership + const proc = await this.backgroundProcessManager.getProcess(processId); + if (!proc) { + return Err(`Process not found: ${processId}`); + } + if (proc.workspaceId !== workspaceId) { + return Err(`Process ${processId} does not belong to workspace ${workspaceId}`); + } + + const result = await this.backgroundProcessManager.terminate(processId); + if (!result.success) { + return Err(result.error); + } + return Ok(undefined); + } + + /** + * Get the tool call IDs of foreground bash processes for a workspace. + * Returns empty array if no foreground bashes are running. + */ + getForegroundToolCallIds(workspaceId: string): string[] { + return this.backgroundProcessManager.getForegroundToolCallIds(workspaceId); + } + + /** + * Send a foreground bash process to background by its tool call ID. + * The process continues running but the agent stops waiting for it. + */ + sendToBackground(toolCallId: string): Result { + const result = this.backgroundProcessManager.sendToBackground(toolCallId); + if (!result.success) { + return Err(result.error); + } + return Ok(undefined); + } + + /** + * Subscribe to background bash state changes. + */ + onBackgroundBashChange(callback: (workspaceId: string) => void): void { + this.backgroundProcessManager.on("change", callback); + } + + /** + * Unsubscribe from background bash state changes. + */ + offBackgroundBashChange(callback: (workspaceId: string) => void): void { + this.backgroundProcessManager.off("change", callback); + } } diff --git a/tests/ipc/backgroundBash.test.ts b/tests/ipc/backgroundBash.test.ts index fd8ebc0530..bf30191ebe 100644 --- a/tests/ipc/backgroundBash.test.ts +++ b/tests/ipc/backgroundBash.test.ts @@ -38,7 +38,8 @@ const BACKGROUND_TOOLS: ToolPolicy = [ const BACKGROUND_TEST_TIMEOUT_MS = 75000; /** - * Extract process ID from bash tool output containing "Background process started with ID: bg-xxx" + * Extract process ID from bash tool output containing "Background process started with ID: xxx" + * The process ID is now the display_name, which can be any string like "Sleep Process" or "bash_123" */ function extractProcessId(events: WorkspaceChatMessage[]): string | null { for (const event of events) { @@ -50,8 +51,9 @@ function extractProcessId(events: WorkspaceChatMessage[]): string | null { ) { const result = (event as { result?: { output?: string } }).result?.output; if (typeof result === "string") { - const match = result.match(/Background process started with ID: (bg-[a-z0-9]+)/); - if (match) return match[1]; + // Match any non-empty process ID after "Background process started with ID: " + const match = result.match(/Background process started with ID: (.+)$/); + if (match) return match[1].trim(); } } } @@ -127,10 +129,10 @@ describeIntegration("Background Bash Execution", () => { 30000 ); - // Extract process ID from tool output + // Extract process ID from tool output (now uses display_name) const processId = extractProcessId(startEvents); expect(processId).not.toBeNull(); - expect(processId).toMatch(/^bg-[a-z0-9]+$/); + expect(processId!.length).toBeGreaterThan(0); // List background processes to verify it's tracked const listEvents = await sendMessageAndWait( diff --git a/tests/ipc/backgroundBashDirect.test.ts b/tests/ipc/backgroundBashDirect.test.ts new file mode 100644 index 0000000000..b7d95e283d --- /dev/null +++ b/tests/ipc/backgroundBashDirect.test.ts @@ -0,0 +1,718 @@ +/** + * Direct integration tests for background bash process manager. + * + * These tests bypass the LLM and call tools directly to verify the service + * wiring is correct. This catches bugs that unit tests miss because unit + * tests create fresh manager instances, while production shares a single + * instance through ServiceContainer. + * + * Key difference from unit tests: + * - Unit tests: Create fresh BackgroundProcessManager per test + * - These tests: Use ServiceContainer's shared BackgroundProcessManager + * + * Key difference from backgroundBash.test.ts: + * - backgroundBash.test.ts: Goes through LLM (slow, flaky, indirect) + * - These tests: Direct tool execution (fast, deterministic, precise) + */ + +import * as fs from "fs/promises"; +import * as path from "path"; +import { createTestEnvironment, cleanupTestEnvironment, type TestEnvironment } from "./setup"; +import { createTempGitRepo, cleanupTempGitRepo, generateBranchName } from "./helpers"; +import { detectDefaultTrunkBranch } from "../../src/node/git"; +import { getToolsForModel } from "../../src/common/utils/tools/tools"; +import { LocalRuntime } from "../../src/node/runtime/LocalRuntime"; +import { BackgroundProcessManager } from "../../src/node/services/backgroundProcessManager"; +import type { InitStateManager } from "../../src/node/services/initStateManager"; + +// Access private fields from ServiceContainer for direct testing +interface ServiceContainerPrivates { + backgroundProcessManager: BackgroundProcessManager; + initStateManager: InitStateManager; +} + +function getBackgroundProcessManager(env: TestEnvironment): BackgroundProcessManager { + return (env.services as unknown as ServiceContainerPrivates).backgroundProcessManager; +} + +function getInitStateManager(env: TestEnvironment): InitStateManager { + return (env.services as unknown as ServiceContainerPrivates).initStateManager; +} + +interface ToolExecuteResult { + success: boolean; + backgroundProcessId?: string; + status?: string; + error?: string; + exitCode?: number; + output?: string; +} + +describe("Background Bash Direct Integration", () => { + let env: TestEnvironment; + let tempGitRepo: string; + let workspaceId: string; + let workspacePath: string; + + beforeAll(async () => { + env = await createTestEnvironment(); + tempGitRepo = await createTempGitRepo(); + + const branchName = generateBranchName("bg-direct-test"); + const trunkBranch = await detectDefaultTrunkBranch(tempGitRepo); + const result = await env.orpc.workspace.create({ + projectPath: tempGitRepo, + branchName, + trunkBranch, + }); + + if (!result.success) { + throw new Error(`Failed to create workspace: ${result.error}`); + } + workspaceId = result.metadata.id; + workspacePath = result.metadata.namedWorkspacePath ?? tempGitRepo; + }); + + afterAll(async () => { + if (workspaceId) { + await env.orpc.workspace.remove({ workspaceId }).catch(() => {}); + } + await cleanupTempGitRepo(tempGitRepo); + await cleanupTestEnvironment(env); + }); + + it("should retrieve output after tools are recreated (multi-message flow)", async () => { + // Simulates production flow where tools are recreated between messages + const manager = getBackgroundProcessManager(env); + const initStateManager = getInitStateManager(env); + const runtime = new LocalRuntime(workspacePath); + const marker = `MULTI_MSG_${Date.now()}`; + + // Message 1: Spawn background process + const tools1 = await getToolsForModel( + "anthropic:claude-sonnet-4-20250514", + { + cwd: workspacePath, + runtime, + secrets: {}, + muxEnv: {}, + runtimeTempDir: "/tmp", + backgroundProcessManager: manager, + workspaceId, + }, + workspaceId, + initStateManager, + {} + ); + + const spawnResult = (await tools1.bash.execute!( + { script: `echo "${marker}"`, run_in_background: true }, + { toolCallId: "spawn", messages: [] } + )) as ToolExecuteResult; + + expect(spawnResult.success).toBe(true); + const processId = spawnResult.backgroundProcessId!; + + await new Promise((resolve) => setTimeout(resolve, 200)); + + // Message 2: Read with NEW tool instances (same manager) + const tools2 = await getToolsForModel( + "anthropic:claude-sonnet-4-20250514", + { + cwd: workspacePath, + runtime, + secrets: {}, + muxEnv: {}, + runtimeTempDir: "/tmp", + backgroundProcessManager: manager, + workspaceId, + }, + workspaceId, + initStateManager, + {} + ); + + const outputResult = (await tools2.bash_output.execute!( + { process_id: processId }, + { toolCallId: "read", messages: [] } + )) as ToolExecuteResult; + + expect(outputResult.success).toBe(true); + expect(outputResult.output).toContain(marker); + }); + + it("should read output files via handle (works for SSH runtime)", async () => { + // Verifies that getOutput uses handle.readOutput() which works for both + // local and SSH runtimes, not direct local fs access + const manager = getBackgroundProcessManager(env); + const runtime = new LocalRuntime(workspacePath); + const testId = `handleread_${Date.now()}`; + const marker = `HANDLE_READ_${testId}`; + + const spawnResult = await manager.spawn(runtime, workspaceId, `echo "${marker}"`, { + cwd: workspacePath, + displayName: testId, + }); + expect(spawnResult.success).toBe(true); + if (!spawnResult.success) return; + + await new Promise((resolve) => setTimeout(resolve, 200)); + + const output = await manager.getOutput(spawnResult.processId); + expect(output.success).toBe(true); + if (output.success) { + expect(output.output).toContain(marker); + } + }); + + it("should support incremental reads", async () => { + const manager = getBackgroundProcessManager(env); + const runtime = new LocalRuntime(workspacePath); + const testId = `incrread_${Date.now()}`; + const marker1 = `INCR_1_${testId}`; + const marker2 = `INCR_2_${testId}`; + + const spawnResult = await manager.spawn( + runtime, + workspaceId, + `echo "${marker1}"; sleep 1; echo "${marker2}"`, + { cwd: workspacePath, displayName: testId } + ); + expect(spawnResult.success).toBe(true); + if (!spawnResult.success) return; + + // Wait longer for output to be flushed to file (CI can be slow) + await new Promise((resolve) => setTimeout(resolve, 500)); + + // First read gets marker1 + const output1 = await manager.getOutput(spawnResult.processId); + expect(output1.success).toBe(true); + if (output1.success) { + expect(output1.output).toContain(marker1); + } + + // Second read immediately - no new content + const output2 = await manager.getOutput(spawnResult.processId); + expect(output2.success).toBe(true); + if (output2.success) { + expect(output2.output).toBe(""); + } + + // Wait for marker2 + await new Promise((resolve) => setTimeout(resolve, 1200)); + + // Third read gets marker2 only + const output3 = await manager.getOutput(spawnResult.processId); + expect(output3.success).toBe(true); + if (output3.success) { + expect(output3.output).toContain(marker2); + expect(output3.output).not.toContain(marker1); + } + }); + + it("should isolate processes by workspace", async () => { + const manager = getBackgroundProcessManager(env); + const runtime = new LocalRuntime(workspacePath); + + // Create second workspace + const branchName2 = generateBranchName("bg-direct-test-2"); + const trunkBranch = await detectDefaultTrunkBranch(tempGitRepo); + const result2 = await env.orpc.workspace.create({ + projectPath: tempGitRepo, + branchName: branchName2, + trunkBranch, + }); + expect(result2.success).toBe(true); + if (!result2.success) return; + const workspaceId2 = result2.metadata.id; + + try { + // Spawn in each workspace + const spawn1 = await manager.spawn(runtime, workspaceId, "echo ws1", { + cwd: workspacePath, + displayName: "test-1", + }); + const spawn2 = await manager.spawn(runtime, workspaceId2, "echo ws2", { + cwd: workspacePath, + displayName: "test-2", + }); + + expect(spawn1.success).toBe(true); + expect(spawn2.success).toBe(true); + if (!spawn1.success || !spawn2.success) return; + + await new Promise((resolve) => setTimeout(resolve, 200)); + + // Cleanup workspace 2 + await manager.cleanup(workspaceId2); + + // Process 1 still accessible + const output1 = await manager.getOutput(spawn1.processId); + expect(output1.success).toBe(true); + + // Process 2 cleaned up + const output2 = await manager.getOutput(spawn2.processId); + expect(output2.success).toBe(false); + } finally { + await env.orpc.workspace.remove({ workspaceId: workspaceId2 }).catch(() => {}); + } + }); +}); + +describe("Background Bash Output Capture", () => { + let env: TestEnvironment; + let tempGitRepo: string; + let workspaceId: string; + let workspacePath: string; + + beforeAll(async () => { + env = await createTestEnvironment(); + tempGitRepo = await createTempGitRepo(); + + const branchName = generateBranchName("bg-output-test"); + const trunkBranch = await detectDefaultTrunkBranch(tempGitRepo); + const result = await env.orpc.workspace.create({ + projectPath: tempGitRepo, + branchName, + trunkBranch, + }); + + if (!result.success) { + throw new Error(`Failed to create workspace: ${result.error}`); + } + workspaceId = result.metadata.id; + workspacePath = result.metadata.namedWorkspacePath ?? tempGitRepo; + }); + + afterAll(async () => { + if (workspaceId) { + await env.orpc.workspace.remove({ workspaceId }).catch(() => {}); + } + await cleanupTempGitRepo(tempGitRepo); + await cleanupTestEnvironment(env); + }); + + it("should capture stderr output when process exits with error", async () => { + // Verifies that stderr is included in unified output + const manager = getBackgroundProcessManager(env); + const runtime = new LocalRuntime(workspacePath); + + // Script that writes to stderr and exits with error + const testId = `stderrerr_${Date.now()}`; + const marker = `ERROR_${testId}`; + const spawnResult = await manager.spawn(runtime, workspaceId, `echo "${marker}" >&2; exit 1`, { + cwd: workspacePath, + displayName: testId, + }); + expect(spawnResult.success).toBe(true); + if (!spawnResult.success) return; + + await new Promise((resolve) => setTimeout(resolve, 300)); + + const output = await manager.getOutput(spawnResult.processId); + expect(output.success).toBe(true); + if (output.success) { + expect(output.exitCode).toBe(1); + expect(output.output).toContain(marker); + } + }); + + it("should capture output when script fails mid-execution", async () => { + const manager = getBackgroundProcessManager(env); + const runtime = new LocalRuntime(workspacePath); + + const testId = `failmid_${Date.now()}`; + const marker1 = `BEFORE_${testId}`; + const marker2 = `ERROR_${testId}`; + // Script that outputs to stdout, then stderr, then continues + const spawnResult = await manager.spawn( + runtime, + workspaceId, + `echo "${marker1}"; echo "${marker2}" >&2; false; echo "NEVER_SEEN"`, + { cwd: workspacePath, displayName: testId } + ); + expect(spawnResult.success).toBe(true); + if (!spawnResult.success) return; + + await new Promise((resolve) => setTimeout(resolve, 300)); + + const output = await manager.getOutput(spawnResult.processId); + expect(output.success).toBe(true); + if (output.success) { + // Both stdout and stderr should be in unified output + expect(output.output).toContain(marker1); + expect(output.output).toContain(marker2); + } + }); + + it("should handle long-running script that outputs to both streams", async () => { + const manager = getBackgroundProcessManager(env); + const runtime = new LocalRuntime(workspacePath); + + const testId = `longrun_${Date.now()}`; + const outMarker = `OUT_${testId}`; + const errMarker = `ERR_${testId}`; + const spawnResult = await manager.spawn( + runtime, + workspaceId, + `for i in 1 2 3; do echo "${outMarker}_$i"; echo "${errMarker}_$i" >&2; done`, + { cwd: workspacePath, displayName: testId } + ); + expect(spawnResult.success).toBe(true); + if (!spawnResult.success) return; + + await new Promise((resolve) => setTimeout(resolve, 500)); + + const output = await manager.getOutput(spawnResult.processId); + expect(output.success).toBe(true); + if (output.success) { + // Unified output should contain both stdout and stderr + expect(output.output).toContain(`${outMarker}_1`); + expect(output.output).toContain(`${outMarker}_3`); + expect(output.output).toContain(`${errMarker}_1`); + expect(output.output).toContain(`${errMarker}_3`); + } + }); +}); + +describe("Foreground to Background Migration", () => { + let env: TestEnvironment; + let tempGitRepo: string; + let workspaceId: string; + let workspacePath: string; + + beforeAll(async () => { + env = await createTestEnvironment(); + tempGitRepo = await createTempGitRepo(); + + const branchName = generateBranchName("fg-to-bg-test"); + const trunkBranch = await detectDefaultTrunkBranch(tempGitRepo); + const result = await env.orpc.workspace.create({ + projectPath: tempGitRepo, + branchName, + trunkBranch, + }); + + if (!result.success) { + throw new Error(`Failed to create workspace: ${result.error}`); + } + workspaceId = result.metadata.id; + workspacePath = result.metadata.namedWorkspacePath ?? tempGitRepo; + }); + + afterAll(async () => { + if (workspaceId) { + await env.orpc.workspace.remove({ workspaceId }).catch(() => {}); + } + await cleanupTempGitRepo(tempGitRepo); + await cleanupTestEnvironment(env); + }); + + it("should migrate foreground bash to background and continue running", async () => { + // This test verifies the complete foregroundβ†’background migration flow: + // 1. Start a foreground bash (run_in_background=false) + // 2. While it's running, call sendToBackground + // 3. The bash tool returns with backgroundProcessId + // 4. Process continues running and output is accessible via bash_output + + const manager = getBackgroundProcessManager(env); + const initStateManager = getInitStateManager(env); + const runtime = new LocalRuntime(workspacePath); + + const testId = `fg_to_bg_${Date.now()}`; + const marker1 = `BEFORE_BG_${testId}`; + const marker2 = `AFTER_BG_${testId}`; + + // Create tools for "message 1" + const tools1 = await getToolsForModel( + "anthropic:claude-sonnet-4-20250514", + { + cwd: workspacePath, + runtime, + secrets: {}, + muxEnv: {}, + runtimeTempDir: "/tmp", + backgroundProcessManager: manager, + workspaceId, + }, + workspaceId, + initStateManager, + {} + ); + + // Start foreground bash that runs for ~3 seconds + // Script: output marker1, sleep, output marker2 + const toolCallId = `tool_${testId}`; + const bashPromise = tools1.bash.execute!( + { + script: `echo "${marker1}"; sleep 2; echo "${marker2}"`, + run_in_background: false, + display_name: testId, + timeout_secs: 30, + }, + { toolCallId, messages: [] } + ) as Promise; + + // Wait for foreground process to register and output first marker + await new Promise((resolve) => setTimeout(resolve, 300)); + + // Verify foreground process is registered + const fgToolCallIds = manager.getForegroundToolCallIds(workspaceId); + expect(fgToolCallIds.includes(toolCallId)).toBe(true); + + // Send to background while running + const bgResult = manager.sendToBackground(toolCallId); + expect(bgResult.success).toBe(true); + + // Wait for bash tool to return (should return immediately after backgrounding) + const result = await bashPromise; + + // Verify result indicates backgrounding (not completion) + expect(result.success).toBe(true); + expect(result.backgroundProcessId).toBe(testId); + // Output so far should contain marker1 + expect(result.output).toContain(marker1); + // Should NOT yet contain marker2 (still running) + expect(result.output).not.toContain(marker2); + + // Foreground registration should be removed + const fgToolCallIds2 = manager.getForegroundToolCallIds(workspaceId); + expect(fgToolCallIds2.includes(toolCallId)).toBe(false); + + // Process should now be in background list + const bgProcs = await manager.list(workspaceId); + const migratedProc = bgProcs.find((p) => p.id === testId); + expect(migratedProc).toBeDefined(); + expect(migratedProc?.status).toBe("running"); + + // === Simulate new message (stream ends, new stream begins) === + // Create NEW tool instances (same manager reference, fresh tools) + const tools2 = await getToolsForModel( + "anthropic:claude-sonnet-4-20250514", + { + cwd: workspacePath, + runtime, + secrets: {}, + muxEnv: {}, + runtimeTempDir: "/tmp", + backgroundProcessManager: manager, + workspaceId, + }, + workspaceId, + initStateManager, + {} + ); + + // Wait for process to complete (marker2 should appear) + await new Promise((resolve) => setTimeout(resolve, 2500)); + + // Get output via bash_output tool (new tool instance) + const outputResult = (await tools2.bash_output.execute!( + { process_id: testId, timeout_secs: 0 }, + { toolCallId: "output_read", messages: [] } + )) as ToolExecuteResult; + + expect(outputResult.success).toBe(true); + // Should now contain marker2 (process continued after migration) + expect(outputResult.output).toContain(marker2); + // Status should be exited (process completed) + expect(outputResult.status).toBe("exited"); + expect(outputResult.exitCode).toBe(0); + }); + + it("should preserve output across stream boundaries", async () => { + // Verifies that output written during foreground phase is preserved + // after migration and accessible in subsequent messages + + const manager = getBackgroundProcessManager(env); + const initStateManager = getInitStateManager(env); + const runtime = new LocalRuntime(workspacePath); + + const testId = `preserve_output_${Date.now()}`; + const marker1 = `EARLY_${testId}`; + const marker2 = `LATE_${testId}`; + + const tools1 = await getToolsForModel( + "anthropic:claude-sonnet-4-20250514", + { + cwd: workspacePath, + runtime, + secrets: {}, + muxEnv: {}, + runtimeTempDir: "/tmp", + backgroundProcessManager: manager, + workspaceId, + }, + workspaceId, + initStateManager, + {} + ); + + const toolCallId = `tool_${testId}`; + // Script outputs marker1, sleeps, then outputs marker2 + const script = `echo "${marker1}"; sleep 2; echo "${marker2}"`; + + const bashPromise = tools1.bash.execute!( + { + script, + run_in_background: false, + display_name: testId, + timeout_secs: 30, + }, + { toolCallId, messages: [] } + ) as Promise; + + // Wait for marker1 to output + await new Promise((resolve) => setTimeout(resolve, 300)); + + // Send to background mid-execution + manager.sendToBackground(toolCallId); + + const result = await bashPromise; + expect(result.success).toBe(true); + expect(result.backgroundProcessId).toBe(testId); + // marker1 should be in the output already + expect(result.output).toContain(marker1); + + // Wait for process to complete + await new Promise((resolve) => setTimeout(resolve, 2500)); + + // Get the full output by reading from the file directly + const proc = await manager.getProcess(testId); + expect(proc).toBeDefined(); + + const outputPath = path.join(proc!.outputDir, "output.log"); + const fullOutput = await fs.readFile(outputPath, "utf-8"); + + // Both markers should be present in the full file + expect(fullOutput).toContain(marker1); + expect(fullOutput).toContain(marker2); + }); + + it("should handle migration when process exits during send", async () => { + // Edge case: process exits right as we try to background it + const manager = getBackgroundProcessManager(env); + const initStateManager = getInitStateManager(env); + const runtime = new LocalRuntime(workspacePath); + + const testId = `fast_exit_${Date.now()}`; + const marker = `QUICK_${testId}`; + + const tools = await getToolsForModel( + "anthropic:claude-sonnet-4-20250514", + { + cwd: workspacePath, + runtime, + secrets: {}, + muxEnv: {}, + runtimeTempDir: "/tmp", + backgroundProcessManager: manager, + workspaceId, + }, + workspaceId, + initStateManager, + {} + ); + + const toolCallId = `tool_${testId}`; + + // Very fast script + const bashPromise = tools.bash.execute!( + { + script: `echo "${marker}"`, + run_in_background: false, + display_name: testId, + timeout_secs: 30, + }, + { toolCallId, messages: [] } + ) as Promise; + + // Small delay then try to background (might already be done) + await new Promise((resolve) => setTimeout(resolve, 50)); + + // This might fail if process already completed - that's fine + manager.sendToBackground(toolCallId); + + const result = await bashPromise; + + // Either it completed normally or was backgrounded + expect(result.success).toBe(true); + expect(result.output).toContain(marker); + }); + + it("should not kill backgrounded process when abort signal fires", async () => { + // Regression test: Previously, when a foreground process was migrated to + // background and then the original stream was aborted (e.g., user sends + // new message), the abort signal would kill the process with exit code -997. + + const manager = getBackgroundProcessManager(env); + const initStateManager = getInitStateManager(env); + const runtime = new LocalRuntime(workspacePath); + + const testId = `abort_after_bg_${Date.now()}`; + const marker1 = `BEFORE_${testId}`; + const marker2 = `AFTER_${testId}`; + + // Create an AbortController to simulate stream abort + const abortController = new AbortController(); + + const tools = await getToolsForModel( + "anthropic:claude-sonnet-4-20250514", + { + cwd: workspacePath, + runtime, + secrets: {}, + muxEnv: {}, + runtimeTempDir: "/tmp", + backgroundProcessManager: manager, + workspaceId, + }, + workspaceId, + initStateManager, + {} + ); + + const toolCallId = `tool_${testId}`; + + // Start a foreground bash with the abort signal + const bashPromise = tools.bash.execute!( + { + script: `echo "${marker1}"; sleep 2; echo "${marker2}"`, + run_in_background: false, + display_name: testId, + timeout_secs: 30, + }, + { toolCallId, messages: [], abortSignal: abortController.signal } + ) as Promise; + + // Wait for first marker + await new Promise((resolve) => setTimeout(resolve, 300)); + + // Send to background + manager.sendToBackground(toolCallId); + const result = await bashPromise; + + expect(result.success).toBe(true); + expect(result.backgroundProcessId).toBe(testId); + + // NOW simulate what happens when user sends a new message: + // The stream manager aborts the previous stream + abortController.abort(); + + // Wait for process to complete (it should NOT be killed by abort) + await new Promise((resolve) => setTimeout(resolve, 2500)); + + // Check process status - should be "exited" with code 0, NOT "killed" with -997 + const proc = await manager.getProcess(testId); + expect(proc).toBeDefined(); + expect(proc?.status).toBe("exited"); + expect(proc?.exitCode).toBe(0); + + // Verify marker2 is in output (process continued to completion) + const output = await manager.getOutput(testId); + expect(output.success).toBe(true); + if (output.success) { + expect(output.output).toContain(marker2); + } + }); +}); diff --git a/tests/runtime/runtime.test.ts b/tests/runtime/runtime.test.ts index afca773356..e1fe6c522e 100644 --- a/tests/runtime/runtime.test.ts +++ b/tests/runtime/runtime.test.ts @@ -17,7 +17,7 @@ import { } from "./ssh-fixture"; import { createTestRuntime, TestWorkspace, type RuntimeType } from "./test-helpers"; import { execBuffered, readFileString, writeFileString } from "@/node/utils/runtime/helpers"; -import type { BackgroundHandle, Runtime } from "@/node/runtime/Runtime"; +import type { Runtime } from "@/node/runtime/Runtime"; import { RuntimeError } from "@/node/runtime/Runtime"; // Skip all tests if TEST_INTEGRATION is not set @@ -1178,205 +1178,6 @@ describeIntegration("Runtime integration tests", () => { } }); }); - - describe("spawnBackground() - Background processes", () => { - // Generate unique IDs for each test to avoid conflicts - const genId = () => `test-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; - - // Polling helpers to handle SSH latency variability - async function waitForOutput( - rt: Runtime, - filePath: string, - opts?: { timeout?: number; interval?: number } - ): Promise { - const { timeout = 5000, interval = 100 } = opts ?? {}; - const start = Date.now(); - while (Date.now() - start < timeout) { - const content = await readFileString(rt, filePath); - if (content.trim()) return content; - await new Promise((r) => setTimeout(r, interval)); - } - return await readFileString(rt, filePath); - } - - async function waitForExitCode( - handle: BackgroundHandle, - opts?: { timeout?: number; interval?: number } - ): Promise { - const { timeout = 5000, interval = 100 } = opts ?? {}; - const start = Date.now(); - while (Date.now() - start < timeout) { - const code = await handle.getExitCode(); - if (code !== null) return code; - await new Promise((r) => setTimeout(r, interval)); - } - return await handle.getExitCode(); - } - - test.concurrent("spawns process and captures output to file", async () => { - const runtime = createRuntime(); - await using workspace = await TestWorkspace.create(runtime, type); - const workspaceId = genId(); - - const result = await runtime.spawnBackground('echo "hello from background"', { - cwd: workspace.path, - workspaceId, - }); - - expect(result.success).toBe(true); - if (!result.success) return; - - expect(result.pid).toBeGreaterThan(0); - expect(result.handle.outputDir).toContain(workspaceId); - expect(result.handle.outputDir).toMatch(/bg-[0-9a-f]{8}/); - - // Poll for output (handles SSH latency) - const stdoutPath = `${result.handle.outputDir}/stdout.log`; - const stdout = await waitForOutput(runtime, stdoutPath); - expect(stdout.trim()).toBe("hello from background"); - - await result.handle.dispose(); - }); - - test.concurrent("captures exit code via trap", async () => { - const runtime = createRuntime(); - await using workspace = await TestWorkspace.create(runtime, type); - const workspaceId = genId(); - - // Spawn a process that exits with code 42 - const result = await runtime.spawnBackground("exit 42", { - cwd: workspace.path, - workspaceId, - }); - - expect(result.success).toBe(true); - if (!result.success) return; - - // Poll for exit code (handles SSH latency) - const exitCode = await waitForExitCode(result.handle); - expect(exitCode).toBe(42); - - await result.handle.dispose(); - }); - - test.concurrent("getExitCode() returns null while process runs", async () => { - const runtime = createRuntime(); - await using workspace = await TestWorkspace.create(runtime, type); - const workspaceId = genId(); - - // Spawn a long-running process - const result = await runtime.spawnBackground("sleep 30", { - cwd: workspace.path, - workspaceId, - }); - - expect(result.success).toBe(true); - if (!result.success) return; - - // Should be running (exit code null) - expect(await result.handle.getExitCode()).toBe(null); - - // Terminate it - await result.handle.terminate(); - - // Poll for exit code after termination - const exitCode = await waitForExitCode(result.handle); - expect(exitCode).not.toBe(null); - - await result.handle.dispose(); - }); - - test.concurrent("terminate() kills running process", async () => { - const runtime = createRuntime(); - await using workspace = await TestWorkspace.create(runtime, type); - const workspaceId = genId(); - - // Spawn a process that runs indefinitely - const result = await runtime.spawnBackground("sleep 60", { - cwd: workspace.path, - workspaceId, - }); - - expect(result.success).toBe(true); - if (!result.success) return; - - // Verify it's running (exit code null) - expect(await result.handle.getExitCode()).toBe(null); - - // Terminate - await result.handle.terminate(); - - // Poll for exit code (handles SSH latency) - const exitCode = await waitForExitCode(result.handle); - expect(exitCode).not.toBe(null); - - await result.handle.dispose(); - }); - - test.concurrent("captures stderr to file", async () => { - const runtime = createRuntime(); - await using workspace = await TestWorkspace.create(runtime, type); - const workspaceId = genId(); - - const result = await runtime.spawnBackground('echo "error message" >&2', { - cwd: workspace.path, - workspaceId, - }); - - expect(result.success).toBe(true); - if (!result.success) return; - - // Poll for output (handles SSH latency) - const stderrPath = `${result.handle.outputDir}/stderr.log`; - const stderr = await waitForOutput(runtime, stderrPath); - expect(stderr.trim()).toBe("error message"); - - await result.handle.dispose(); - }); - - test.concurrent("respects working directory", async () => { - const runtime = createRuntime(); - await using workspace = await TestWorkspace.create(runtime, type); - const workspaceId = genId(); - - const result = await runtime.spawnBackground("pwd", { - cwd: workspace.path, - workspaceId, - }); - - expect(result.success).toBe(true); - if (!result.success) return; - - // Poll for output (handles SSH latency) - const stdoutPath = `${result.handle.outputDir}/stdout.log`; - const stdout = await waitForOutput(runtime, stdoutPath); - expect(stdout.trim()).toBe(workspace.path); - - await result.handle.dispose(); - }); - - test.concurrent("passes environment variables", async () => { - const runtime = createRuntime(); - await using workspace = await TestWorkspace.create(runtime, type); - const workspaceId = genId(); - - const result = await runtime.spawnBackground('echo "secret=$MY_SECRET"', { - cwd: workspace.path, - workspaceId, - env: { MY_SECRET: "hunter2" }, - }); - - expect(result.success).toBe(true); - if (!result.success) return; - - // Poll for output (handles SSH latency) - const stdoutPath = `${result.handle.outputDir}/stdout.log`; - const stdout = await waitForOutput(runtime, stdoutPath); - expect(stdout.trim()).toBe("secret=hunter2"); - - await result.handle.dispose(); - }); - }); } ); }); diff --git a/tests/runtime/test-helpers.ts b/tests/runtime/test-helpers.ts index a00ef652cb..9b73c5085f 100644 --- a/tests/runtime/test-helpers.ts +++ b/tests/runtime/test-helpers.ts @@ -30,7 +30,7 @@ export function createTestRuntime( // Resolve symlinks (e.g., /tmp -> /private/tmp on macOS) to match git worktree paths // Note: "local" in tests means WorktreeRuntime (isolated git worktrees) const resolvedWorkdir = realpathSync(workdir); - return new WorktreeRuntime(resolvedWorkdir, resolvedWorkdir); + return new WorktreeRuntime(resolvedWorkdir); case "ssh": if (!sshConfig) { throw new Error("SSH config required for SSH runtime");