diff --git a/packages/cli/src/services/BuiltinCommandLoader.ts b/packages/cli/src/services/BuiltinCommandLoader.ts index 9d7fd4722..71a591ebb 100644 --- a/packages/cli/src/services/BuiltinCommandLoader.ts +++ b/packages/cli/src/services/BuiltinCommandLoader.ts @@ -8,6 +8,7 @@ import type { ICommandLoader } from './types.js'; import type { SlashCommand } from '../ui/commands/types.js'; import type { Config } from '@qwen-code/qwen-code-core'; import { aboutCommand } from '../ui/commands/aboutCommand.js'; +import { bashesCommand } from '../ui/commands/bashesCommand.js'; import { agentsCommand } from '../ui/commands/agentsCommand.js'; import { arenaCommand } from '../ui/commands/arenaCommand.js'; import { approvalModeCommand } from '../ui/commands/approvalModeCommand.js'; @@ -91,6 +92,7 @@ export class BuiltinCommandLoader implements ICommandLoader { const allDefinitions: Array = [ aboutCommand, agentsCommand, + bashesCommand, arenaCommand, approvalModeCommand, authCommand, diff --git a/packages/cli/src/ui/commands/bashesCommand.test.ts b/packages/cli/src/ui/commands/bashesCommand.test.ts new file mode 100644 index 000000000..9f12aff83 --- /dev/null +++ b/packages/cli/src/ui/commands/bashesCommand.test.ts @@ -0,0 +1,94 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { vi, describe, it, expect, beforeEach } from 'vitest'; +import { bashesCommand } from './bashesCommand.js'; +import { type CommandContext } from './types.js'; +import { createMockCommandContext } from '../../test-utils/mockCommandContext.js'; +import type { BackgroundShellEntry } from '@qwen-code/qwen-code-core'; + +function entry( + overrides: Partial = {}, +): BackgroundShellEntry { + return { + shellId: 'bg_aaaaaaaa', + command: 'sleep 60', + cwd: '/tmp', + status: 'running', + startTime: Date.now() - 5_000, + outputPath: '/tmp/tasks/sess/shell-bg_aaaaaaaa.output', + abortController: new AbortController(), + ...overrides, + }; +} + +describe('bashesCommand', () => { + let context: CommandContext; + let getAll: ReturnType; + + beforeEach(() => { + getAll = vi.fn().mockReturnValue([]); + context = createMockCommandContext({ + services: { + config: { + getBackgroundShellRegistry: () => ({ getAll }), + }, + }, + } as unknown as Parameters[0]); + }); + + it('reports an empty registry', async () => { + const result = await bashesCommand.action!(context, ''); + expect(result).toEqual({ + type: 'message', + messageType: 'info', + content: 'No background shells.', + }); + }); + + it('lists running and terminal entries with status / runtime / output path', async () => { + getAll.mockReturnValue([ + entry({ + shellId: 'bg_run', + command: 'npm run dev', + status: 'running', + startTime: Date.now() - 12_000, + pid: 1111, + }), + entry({ + shellId: 'bg_done', + command: 'npm test', + status: 'completed', + exitCode: 0, + startTime: Date.now() - 70_000, + endTime: Date.now() - 5_000, + outputPath: '/tmp/tasks/sess/shell-bg_done.output', + }), + entry({ + shellId: 'bg_fail', + command: 'flaky.sh', + status: 'failed', + error: 'spawn ENOENT', + startTime: Date.now() - 3_000, + endTime: Date.now() - 2_000, + }), + ]); + + const result = await bashesCommand.action!(context, ''); + if (!result || result.type !== 'message') { + throw new Error('expected message result'); + } + expect(result.content).toContain('Background shells (3 total)'); + expect(result.content).toContain('[bg_run] running'); + expect(result.content).toContain('pid=1111'); + expect(result.content).toContain('npm run dev'); + expect(result.content).toContain('[bg_done] completed (exit 0)'); + expect(result.content).toContain('[bg_fail] failed: spawn ENOENT'); + expect(result.content).toContain( + 'output: /tmp/tasks/sess/shell-bg_done.output', + ); + }); +}); diff --git a/packages/cli/src/ui/commands/bashesCommand.ts b/packages/cli/src/ui/commands/bashesCommand.ts new file mode 100644 index 000000000..764c91341 --- /dev/null +++ b/packages/cli/src/ui/commands/bashesCommand.ts @@ -0,0 +1,85 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { BackgroundShellEntry } from '@qwen-code/qwen-code-core'; +import type { SlashCommand } from './types.js'; +import { CommandKind } from './types.js'; +import { t } from '../../i18n/index.js'; + +function formatRuntime(ms: number): string { + if (ms < 0) ms = 0; + const seconds = Math.floor(ms / 1000); + if (seconds < 60) return `${seconds}s`; + const minutes = Math.floor(seconds / 60); + const remainingSeconds = seconds % 60; + return `${minutes}m${remainingSeconds.toString().padStart(2, '0')}s`; +} + +function statusLabel(entry: BackgroundShellEntry): string { + switch (entry.status) { + case 'completed': + return `completed (exit ${entry.exitCode ?? '?'})`; + case 'failed': + return `failed: ${entry.error ?? 'unknown error'}`; + case 'cancelled': + return 'cancelled'; + case 'running': + return 'running'; + default: + return entry.status; + } +} + +export const bashesCommand: SlashCommand = { + name: 'bashes', + altNames: ['shells'], + get description() { + return t('List background shells started via the shell tool'); + }, + kind: CommandKind.BUILT_IN, + supportedModes: ['interactive', 'non_interactive', 'acp'] as const, + action: async (context) => { + const { config } = context.services; + if (!config) { + return { + type: 'message' as const, + messageType: 'error' as const, + content: 'Config not available.', + }; + } + + const entries = config.getBackgroundShellRegistry().getAll(); + + if (entries.length === 0) { + return { + type: 'message' as const, + messageType: 'info' as const, + content: 'No background shells.', + }; + } + + const now = Date.now(); + const lines: string[] = [ + `Background shells (${entries.length} total)`, + '', + ]; + for (const entry of entries) { + const endTime = entry.endTime ?? now; + const runtime = formatRuntime(endTime - entry.startTime); + const pidPart = entry.pid !== undefined ? ` pid=${entry.pid}` : ''; + lines.push( + `[${entry.shellId}] ${statusLabel(entry)} ${runtime}${pidPart} ${entry.command}`, + ); + lines.push(` output: ${entry.outputPath}`); + } + + return { + type: 'message' as const, + messageType: 'info' as const, + content: lines.join('\n'), + }; + }, +}; diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index de4d151b1..cacc33026 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -61,6 +61,7 @@ import { PermissionManager } from '../permissions/permission-manager.js'; import { SubagentManager } from '../subagents/subagent-manager.js'; import type { SubagentConfig } from '../subagents/types.js'; import { BackgroundTaskRegistry } from '../agents/background-tasks.js'; +import { BackgroundShellRegistry } from '../services/backgroundShellRegistry.js'; import { DEFAULT_OTLP_ENDPOINT, DEFAULT_TELEMETRY_TARGET, @@ -544,6 +545,7 @@ export class Config { private promptRegistry!: PromptRegistry; private subagentManager!: SubagentManager; private readonly backgroundTaskRegistry = new BackgroundTaskRegistry(); + private readonly backgroundShellRegistry = new BackgroundShellRegistry(); private extensionManager!: ExtensionManager; private skillManager: SkillManager | null = null; private permissionManager: PermissionManager | null = null; @@ -2467,6 +2469,10 @@ export class Config { return this.backgroundTaskRegistry; } + getBackgroundShellRegistry(): BackgroundShellRegistry { + return this.backgroundShellRegistry; + } + /** * Whether interactive permission prompts should be auto-denied. * True for background agents that have no UI to show prompts. diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 80210ee03..fcb93170d 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -147,6 +147,7 @@ export * from './services/sessionService.js'; export * from './services/sessionTitle.js'; export { stripTerminalControlSequences } from './utils/terminalSafe.js'; export * from './services/shellExecutionService.js'; +export * from './services/backgroundShellRegistry.js'; export * from './utils/bareMode.js'; // ============================================================================ diff --git a/packages/core/src/services/backgroundShellRegistry.test.ts b/packages/core/src/services/backgroundShellRegistry.test.ts new file mode 100644 index 000000000..836b4578b --- /dev/null +++ b/packages/core/src/services/backgroundShellRegistry.test.ts @@ -0,0 +1,132 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { + BackgroundShellRegistry, + type BackgroundShellEntry, +} from './backgroundShellRegistry.js'; + +function makeEntry( + overrides: Partial = {}, +): BackgroundShellEntry { + return { + shellId: 's1', + command: 'sleep 60', + cwd: '/tmp', + status: 'running', + startTime: 1000, + outputPath: '/tmp/s1.output', + abortController: new AbortController(), + ...overrides, + }; +} + +describe('BackgroundShellRegistry', () => { + describe('register / get / getAll', () => { + it('round-trips a registered entry by id', () => { + const reg = new BackgroundShellRegistry(); + const e = makeEntry({ shellId: 'a' }); + reg.register(e); + expect(reg.get('a')).toBe(e); + }); + + it('returns undefined for unknown id', () => { + const reg = new BackgroundShellRegistry(); + expect(reg.get('missing')).toBeUndefined(); + }); + + it('lists all entries via getAll', () => { + const reg = new BackgroundShellRegistry(); + const a = makeEntry({ shellId: 'a' }); + const b = makeEntry({ shellId: 'b' }); + reg.register(a); + reg.register(b); + const all = reg.getAll(); + expect(all).toHaveLength(2); + expect(all).toContain(a); + expect(all).toContain(b); + }); + }); + + describe('complete', () => { + it('transitions running → completed with exitCode and endTime', () => { + const reg = new BackgroundShellRegistry(); + reg.register(makeEntry({ shellId: 'a' })); + reg.complete('a', 0, 2000); + const e = reg.get('a')!; + expect(e.status).toBe('completed'); + expect(e.exitCode).toBe(0); + expect(e.endTime).toBe(2000); + }); + + it('is a no-op when entry is not running', () => { + const reg = new BackgroundShellRegistry(); + reg.register(makeEntry({ shellId: 'a' })); + reg.cancel('a', 1500); + reg.complete('a', 0, 2000); + const e = reg.get('a')!; + expect(e.status).toBe('cancelled'); + expect(e.exitCode).toBeUndefined(); + }); + + it('is a no-op for unknown id', () => { + const reg = new BackgroundShellRegistry(); + expect(() => reg.complete('missing', 0, 0)).not.toThrow(); + }); + }); + + describe('fail', () => { + it('transitions running → failed with error and endTime', () => { + const reg = new BackgroundShellRegistry(); + reg.register(makeEntry({ shellId: 'a' })); + reg.fail('a', 'spawn error', 2000); + const e = reg.get('a')!; + expect(e.status).toBe('failed'); + expect(e.error).toBe('spawn error'); + expect(e.endTime).toBe(2000); + }); + + it('is a no-op when entry is not running', () => { + const reg = new BackgroundShellRegistry(); + reg.register(makeEntry({ shellId: 'a' })); + reg.complete('a', 0, 1500); + reg.fail('a', 'late error', 2000); + const e = reg.get('a')!; + expect(e.status).toBe('completed'); + expect(e.error).toBeUndefined(); + }); + }); + + describe('cancel', () => { + it('transitions running → cancelled and aborts the signal', () => { + const reg = new BackgroundShellRegistry(); + const ac = new AbortController(); + reg.register(makeEntry({ shellId: 'a', abortController: ac })); + reg.cancel('a', 2000); + const e = reg.get('a')!; + expect(e.status).toBe('cancelled'); + expect(e.endTime).toBe(2000); + expect(ac.signal.aborted).toBe(true); + }); + + it('is a no-op when entry is already terminal', () => { + const reg = new BackgroundShellRegistry(); + const ac = new AbortController(); + reg.register(makeEntry({ shellId: 'a', abortController: ac })); + reg.complete('a', 0, 1500); + reg.cancel('a', 2000); + const e = reg.get('a')!; + expect(e.status).toBe('completed'); + expect(ac.signal.aborted).toBe(false); + }); + + it('is a no-op for unknown id', () => { + const reg = new BackgroundShellRegistry(); + expect(() => reg.cancel('missing', 0)).not.toThrow(); + }); + }); +}); diff --git a/packages/core/src/services/backgroundShellRegistry.ts b/packages/core/src/services/backgroundShellRegistry.ts new file mode 100644 index 000000000..9221dc4fb --- /dev/null +++ b/packages/core/src/services/backgroundShellRegistry.ts @@ -0,0 +1,88 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Tracks background shell processes spawned via the `shell` tool with + * `is_background: true`. Each entry holds the metadata the agent and the + * `/bashes` slash command need to query, observe, or terminate a running + * background shell. + * + * State machine: register → running → { completed | failed | cancelled }. + * Transitions out of running are one-shot: complete/fail/cancel become + * no-ops once the entry has settled. This prevents late callbacks (e.g. a + * process that exits during cancellation) from clobbering the terminal + * status. + */ + +export type BackgroundShellStatus = + | 'running' + | 'completed' + | 'failed' + | 'cancelled'; + +export interface BackgroundShellEntry { + /** Stable id used by the model and the `/bashes` UI. */ + shellId: string; + /** The user-supplied command, after any pre-processing the tool applies. */ + command: string; + /** Working directory the process was spawned in. */ + cwd: string; + /** OS pid once spawned; absent if registration happens before spawn. */ + pid?: number; + status: BackgroundShellStatus; + /** Exit code on `completed`. */ + exitCode?: number; + /** Error message on `failed`. */ + error?: string; + /** ms epoch when the entry was registered. */ + startTime: number; + /** ms epoch when the entry transitioned out of running. */ + endTime?: number; + /** Absolute path of the captured stdout/stderr file. */ + outputPath: string; + /** Aborted by `cancel()`; callers should wire it into the spawn. */ + abortController: AbortController; +} + +export class BackgroundShellRegistry { + private readonly entries = new Map(); + + register(entry: BackgroundShellEntry): void { + this.entries.set(entry.shellId, entry); + } + + get(shellId: string): BackgroundShellEntry | undefined { + return this.entries.get(shellId); + } + + getAll(): readonly BackgroundShellEntry[] { + return [...this.entries.values()]; + } + + complete(shellId: string, exitCode: number, endTime: number): void { + const entry = this.entries.get(shellId); + if (!entry || entry.status !== 'running') return; + entry.status = 'completed'; + entry.exitCode = exitCode; + entry.endTime = endTime; + } + + fail(shellId: string, error: string, endTime: number): void { + const entry = this.entries.get(shellId); + if (!entry || entry.status !== 'running') return; + entry.status = 'failed'; + entry.error = error; + entry.endTime = endTime; + } + + cancel(shellId: string, endTime: number): void { + const entry = this.entries.get(shellId); + if (!entry || entry.status !== 'running') return; + entry.status = 'cancelled'; + entry.endTime = endTime; + entry.abortController.abort(); + } +} diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index a8748e375..d27d2c5aa 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -31,8 +31,6 @@ import { } from '../services/shellExecutionService.js'; import * as fs from 'node:fs'; import * as os from 'node:os'; -import { EOL } from 'node:os'; -import * as path from 'node:path'; import * as crypto from 'node:crypto'; import { ToolErrorType } from './tool-error.js'; import { OUTPUT_UPDATE_INTERVAL_MS } from './shell.js'; @@ -55,12 +53,14 @@ describe('ShellTool', () => { getPermissionsDeny: vi.fn().mockReturnValue([]), getDebugMode: vi.fn().mockReturnValue(false), getTargetDir: vi.fn().mockReturnValue('/test/dir'), + getSessionId: vi.fn().mockReturnValue('test-session'), getWorkspaceContext: vi .fn() .mockReturnValue(createMockWorkspaceContext('/test/dir')), storage: { getUserSkillsDirs: vi.fn().mockReturnValue(['/test/dir/.qwen/skills']), getProjectTempDir: vi.fn().mockReturnValue('/tmp/qwen-temp'), + getProjectDir: vi.fn().mockReturnValue('/test/proj'), }, getTruncateToolOutputThreshold: vi.fn().mockReturnValue(0), getTruncateToolOutputLines: vi.fn().mockReturnValue(0), @@ -72,8 +72,23 @@ describe('ShellTool', () => { email: 'qwen-coder@alibabacloud.com', }), getShouldUseNodePtyShell: vi.fn().mockReturnValue(false), + getBackgroundShellRegistry: vi.fn().mockReturnValue({ + register: vi.fn(), + get: vi.fn(), + getAll: vi.fn().mockReturnValue([]), + cancel: vi.fn(), + complete: vi.fn(), + fail: vi.fn(), + }), } as unknown as Config; + // executeBackground writes to disk; stub mkdirSync + createWriteStream. + vi.mocked(fs.mkdirSync).mockReturnValue(undefined); + vi.mocked(fs.createWriteStream).mockReturnValue({ + write: vi.fn(), + end: vi.fn(), + } as unknown as fs.WriteStream); + shellTool = new ShellTool(mockConfig); vi.mocked(os.platform).mockReturnValue('linux'); @@ -271,81 +286,129 @@ describe('ShellTool', () => { resolveExecutionPromise(fullResult); }; - it('should wrap background command on linux and parse pgrep output', async () => { + it('runs background commands as managed pool entries (no & / pgrep wrap)', async () => { + const registry = mockConfig.getBackgroundShellRegistry(); const invocation = shellTool.build({ - command: 'my-command', + command: 'npm start', is_background: true, }); - const promise = invocation.execute(mockAbortSignal); - resolveShellExecution({ pid: 54321 }); - vi.mocked(fs.existsSync).mockReturnValue(true); - vi.mocked(fs.readFileSync).mockReturnValue(`54321${EOL}54322${EOL}`); // Service PID and background PID + const result = await invocation.execute(mockAbortSignal); - const result = await promise; - - const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp'); - const wrappedCommand = `{ my-command & }; __code=$?; pgrep -g 0 >${tmpFile} 2>&1; exit $__code;`; + // Spawn happens with the unwrapped command — no '&', no pgrep envelope. expect(mockShellExecutionService).toHaveBeenCalledWith( - wrappedCommand, + 'npm start', '/test/dir', expect.any(Function), expect.any(AbortSignal), false, {}, ); - expect(result.llmContent).toContain('PIDs: 54322'); - expect(vi.mocked(fs.unlinkSync)).toHaveBeenCalledWith(tmpFile); + // Entry registered with the spawn pid. + expect(registry.register).toHaveBeenCalledTimes(1); + const entry = (registry.register as Mock).mock.calls[0][0]; + expect(entry.command).toBe('npm start'); + expect(entry.cwd).toBe('/test/dir'); + expect(entry.status).toBe('running'); + expect(entry.pid).toBe(12345); + expect(typeof entry.shellId).toBe('string'); + expect(entry.outputPath).toContain('shell-'); + // Returns immediately with id + output path; agent's turn isn't blocked. + expect(result.llmContent).toContain(entry.shellId); + expect(result.llmContent).toContain(entry.outputPath); }); - it('should add ampersand to command when is_background is true and command does not end with &', async () => { + it('settles a background entry as completed when the process exits cleanly', async () => { + const registry = mockConfig.getBackgroundShellRegistry(); const invocation = shellTool.build({ - command: 'npm start', + command: 'true', is_background: true, }); - const promise = invocation.execute(mockAbortSignal); - resolveShellExecution({ pid: 54321 }); + await invocation.execute(mockAbortSignal); + const entry = (registry.register as Mock).mock.calls[0][0]; - vi.mocked(fs.existsSync).mockReturnValue(true); - vi.mocked(fs.readFileSync).mockReturnValue('54321\n54322\n'); + resolveExecutionPromise({ + rawOutput: Buffer.from(''), + output: '', + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + }); + // Flush the .then() microtask attached to resultPromise. + await new Promise((r) => setImmediate(r)); - await promise; - - const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp'); - const wrappedCommand = `{ npm start & }; __code=$?; pgrep -g 0 >${tmpFile} 2>&1; exit $__code;`; - expect(mockShellExecutionService).toHaveBeenCalledWith( - wrappedCommand, - expect.any(String), - expect.any(Function), - expect.any(AbortSignal), - false, - {}, + expect(registry.complete).toHaveBeenCalledWith( + entry.shellId, + 0, + expect.any(Number), ); + expect(registry.fail).not.toHaveBeenCalled(); + expect(registry.cancel).not.toHaveBeenCalled(); }); - it('should not add extra ampersand when is_background is true and command already ends with &', async () => { + it('settles a background entry as failed when ShellExecutionService reports error', async () => { + const registry = mockConfig.getBackgroundShellRegistry(); const invocation = shellTool.build({ - command: 'npm start &', + command: 'no-such-command', is_background: true, }); - const promise = invocation.execute(mockAbortSignal); - resolveShellExecution({ pid: 54321 }); + await invocation.execute(mockAbortSignal); + const entry = (registry.register as Mock).mock.calls[0][0]; - vi.mocked(fs.existsSync).mockReturnValue(true); - vi.mocked(fs.readFileSync).mockReturnValue('54321\n54322\n'); + resolveExecutionPromise({ + rawOutput: Buffer.from(''), + output: '', + exitCode: null, + signal: null, + error: new Error('spawn ENOENT'), + aborted: false, + pid: 12345, + executionMethod: 'child_process', + }); + await new Promise((r) => setImmediate(r)); - await promise; - - const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp'); - const wrappedCommand = `{ npm start & }; __code=$?; pgrep -g 0 >${tmpFile} 2>&1; exit $__code;`; - expect(mockShellExecutionService).toHaveBeenCalledWith( - wrappedCommand, - expect.any(String), - expect.any(Function), - expect.any(AbortSignal), - false, - {}, + expect(registry.fail).toHaveBeenCalledWith( + entry.shellId, + 'spawn ENOENT', + expect.any(Number), ); + expect(registry.complete).not.toHaveBeenCalled(); + }); + + it('settles a background entry as cancelled when the abort signal fires', async () => { + const registry = mockConfig.getBackgroundShellRegistry(); + const ac = new AbortController(); + const invocation = shellTool.build({ + command: 'sleep 99', + is_background: true, + }); + await invocation.execute(ac.signal); + const entry = (registry.register as Mock).mock.calls[0][0]; + // Simulate registry already setting status to 'running' before cancel hits. + (registry.get as Mock).mockReturnValue({ status: 'running' }); + + ac.abort(); + resolveExecutionPromise({ + rawOutput: Buffer.from(''), + output: '', + exitCode: null, + signal: 'SIGTERM', + error: null, + aborted: true, + pid: 12345, + executionMethod: 'child_process', + }); + await new Promise((r) => setImmediate(r)); + + expect(registry.cancel).toHaveBeenCalledWith( + entry.shellId, + expect.any(Number), + ); + expect(registry.complete).not.toHaveBeenCalled(); + expect(registry.fail).not.toHaveBeenCalled(); }); it('should not add ampersand when is_background is false', async () => { @@ -479,23 +542,6 @@ describe('ShellTool', () => { ).toThrow('Directory must be an absolute path.'); }); - it('should clean up the temp file on synchronous execution error', async () => { - const error = new Error('sync spawn error'); - mockShellExecutionService.mockImplementation(() => { - throw error; - }); - vi.mocked(fs.existsSync).mockReturnValue(true); // Pretend the file exists - - const invocation = shellTool.build({ - command: 'a-command', - is_background: false, - }); - await expect(invocation.execute(mockAbortSignal)).rejects.toThrow(error); - - const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp'); - expect(vi.mocked(fs.unlinkSync)).toHaveBeenCalledWith(tmpFile); - }); - describe('Streaming to `updateOutput`', () => { let updateOutputMock: Mock; beforeEach(() => { @@ -1016,43 +1062,6 @@ describe('ShellTool', () => { }); }); - describe('Windows background execution', () => { - it('should clean up trailing ampersand on Windows for background tasks', async () => { - vi.mocked(os.platform).mockReturnValue('win32'); - const mockAbortSignal = new AbortController().signal; - - const invocation = shellTool.build({ - command: 'npm start &', - is_background: true, - }); - - const promise = invocation.execute(mockAbortSignal); - - // Simulate immediate success (process started) - resolveExecutionPromise({ - rawOutput: Buffer.from(''), - output: '', - exitCode: 0, - signal: null, - error: null, - aborted: false, - pid: 12345, - executionMethod: 'child_process', - }); - - await promise; - - expect(mockShellExecutionService).toHaveBeenCalledWith( - 'npm start', - expect.any(String), - expect.any(Function), - expect.any(AbortSignal), - false, - {}, - ); - }); - }); - describe('timeout parameter', () => { it('should validate timeout parameter correctly', async () => { // Valid timeout @@ -1177,40 +1186,6 @@ describe('ShellTool', () => { expect(calledSignal).not.toBe(mockAbortSignal); }); - it('should not create timeout signal for background execution', async () => { - const mockAbortSignal = new AbortController().signal; - const invocation = shellTool.build({ - command: 'npm start', - is_background: true, - timeout: 5000, - }); - - const promise = invocation.execute(mockAbortSignal); - - resolveExecutionPromise({ - rawOutput: Buffer.from(''), - output: 'Background command started. PID: 12345', - exitCode: 0, - signal: null, - error: null, - aborted: false, - pid: 12345, - executionMethod: 'child_process', - }); - - await promise; - - // For background execution, the original signal should be used - expect(mockShellExecutionService).toHaveBeenCalledWith( - expect.any(String), - expect.any(String), - expect.any(Function), - mockAbortSignal, - false, - {}, - ); - }); - it('should handle timeout vs user cancellation correctly', async () => { const userAbortController = new AbortController(); const invocation = shellTool.build({ diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 02c486181..2438612ec 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -6,7 +6,7 @@ import fs from 'node:fs'; import path from 'node:path'; -import os, { EOL } from 'node:os'; +import os from 'node:os'; import crypto from 'node:crypto'; import type { Config } from '../config/config.js'; import { ToolNames, ToolDisplayNames } from './tool-names.js'; @@ -29,6 +29,7 @@ import type { ShellOutputEvent, } from '../services/shellExecutionService.js'; import { ShellExecutionService } from '../services/shellExecutionService.js'; +import type { BackgroundShellEntry } from '../services/backgroundShellRegistry.js'; import { formatMemoryUsage } from '../utils/formatters.js'; import type { AnsiOutput } from '../utils/terminalSerializer.js'; import { isSubpaths } from '../utils/paths.js'; @@ -208,9 +209,12 @@ export class ShellToolInvocation extends BaseToolInvocation< }; } - const effectiveTimeout = this.params.is_background - ? undefined - : (this.params.timeout ?? DEFAULT_FOREGROUND_TIMEOUT_MS); + if (this.params.is_background) { + return this.executeBackground(signal, shellExecutionConfig); + } + + const effectiveTimeout = + this.params.timeout ?? DEFAULT_FOREGROUND_TIMEOUT_MS; // Create combined signal with timeout for foreground execution let combinedSignal = signal; @@ -219,294 +223,292 @@ export class ShellToolInvocation extends BaseToolInvocation< combinedSignal = AbortSignal.any([signal, timeoutSignal]); } - const isWindows = os.platform() === 'win32'; - const tempFileName = `shell_pgrep_${crypto - .randomBytes(6) - .toString('hex')}.tmp`; - const tempFilePath = path.join(os.tmpdir(), tempFileName); + // Add co-author to git commit commands + const processedCommand = this.addCoAuthorToGitCommit(strippedCommand); + const commandToExecute = processedCommand; + const cwd = this.params.directory || this.config.getTargetDir(); - try { - // Add co-author to git commit commands - const processedCommand = this.addCoAuthorToGitCommit(strippedCommand); + let cumulativeOutput: string | AnsiOutput = ''; + let lastUpdateTime = Date.now(); + let isBinaryStream = false; + let totalLines = 0; + let totalBytes = 0; - const shouldRunInBackground = this.params.is_background; - let finalCommand = processedCommand; + const { result: resultPromise, pid } = await ShellExecutionService.execute( + commandToExecute, + cwd, + (event: ShellOutputEvent) => { + let shouldUpdate = false; - // On non-Windows, use & to run in background. - // On Windows, we don't use start /B because it creates a detached process that - // doesn't die when the parent dies. Instead, we rely on the race logic below - // to return early while keeping the process attached (detached: false). - if ( - !isWindows && - shouldRunInBackground && - !finalCommand.trim().endsWith('&') - ) { - finalCommand = finalCommand.trim() + ' &'; - } - - // On Windows, we rely on the race logic below to handle background tasks. - // We just ensure the command string is clean. - if (isWindows && shouldRunInBackground) { - finalCommand = finalCommand.trim().replace(/&+$/, '').trim(); - } - - // On non-Windows background commands, wrap with pgrep to capture - // subprocess PIDs so we can report them to the user. - const commandToExecute = - !isWindows && shouldRunInBackground - ? (() => { - let command = finalCommand.trim(); - if (!command.endsWith('&')) command += ';'; - return `{ ${command} }; __code=$?; pgrep -g 0 >${tempFilePath} 2>&1; exit $__code;`; - })() - : finalCommand; - - const cwd = this.params.directory || this.config.getTargetDir(); - - let cumulativeOutput: string | AnsiOutput = ''; - let lastUpdateTime = Date.now(); - let isBinaryStream = false; - let totalLines = 0; - let totalBytes = 0; - - const { result: resultPromise, pid } = - await ShellExecutionService.execute( - commandToExecute, - cwd, - (event: ShellOutputEvent) => { - let shouldUpdate = false; - - switch (event.type) { - case 'data': - if (isBinaryStream) break; - cumulativeOutput = event.chunk; - // Stats are only consumed by the ANSI-output branch below, - // so skip the per-chunk accounting for plain string chunks. - if (Array.isArray(event.chunk)) { - totalLines = event.chunk.length; - totalBytes = event.chunk.reduce( - (sum, line) => - sum + - line.reduce( - (ls, token) => - ls + Buffer.byteLength(token.text, 'utf-8'), - 0, - ), + switch (event.type) { + case 'data': + if (isBinaryStream) break; + cumulativeOutput = event.chunk; + // Stats are only consumed by the ANSI-output branch below, + // so skip the per-chunk accounting for plain string chunks. + if (Array.isArray(event.chunk)) { + totalLines = event.chunk.length; + totalBytes = event.chunk.reduce( + (sum, line) => + sum + + line.reduce( + (ls, token) => ls + Buffer.byteLength(token.text, 'utf-8'), 0, - ); - } - shouldUpdate = true; - break; - case 'binary_detected': - isBinaryStream = true; - cumulativeOutput = - '[Binary output detected. Halting stream...]'; - shouldUpdate = true; - break; - case 'binary_progress': - isBinaryStream = true; - cumulativeOutput = `[Receiving binary output... ${formatMemoryUsage( - event.bytesReceived, - )} received]`; - if (Date.now() - lastUpdateTime > OUTPUT_UPDATE_INTERVAL_MS) { - shouldUpdate = true; - } - break; - default: { - throw new Error('An unhandled ShellOutputEvent was found.'); - } + ), + 0, + ); } - - if (shouldUpdate && updateOutput) { - if (typeof cumulativeOutput === 'string') { - updateOutput(cumulativeOutput); - } else { - updateOutput({ - ansiOutput: cumulativeOutput, - totalLines, - totalBytes, - // Only include timeout when user explicitly set it - ...(this.params.timeout != null && { - timeoutMs: this.params.timeout, - }), - }); - } - lastUpdateTime = Date.now(); + shouldUpdate = true; + break; + case 'binary_detected': + isBinaryStream = true; + cumulativeOutput = '[Binary output detected. Halting stream...]'; + shouldUpdate = true; + break; + case 'binary_progress': + isBinaryStream = true; + cumulativeOutput = `[Receiving binary output... ${formatMemoryUsage( + event.bytesReceived, + )} received]`; + if (Date.now() - lastUpdateTime > OUTPUT_UPDATE_INTERVAL_MS) { + shouldUpdate = true; } - }, - combinedSignal, - shouldRunInBackground - ? false - : this.config.getShouldUseNodePtyShell(), - shellExecutionConfig ?? {}, - ); - - if (pid && setPidCallback) { - setPidCallback(pid); - } - - // On Windows, background commands rely on early return since there's - // no & backgrounding or pgrep. Awaiting would block until completion. - if (shouldRunInBackground && isWindows) { - const pidMsg = pid ? ` PID: ${pid}` : ''; - const killHint = ' (Use taskkill /F /T /PID to stop)'; - - return { - llmContent: `Background command started.${pidMsg}${killHint}`, - returnDisplay: `Background command started.${pidMsg}${killHint}`, - }; - } - - const result = await resultPromise; - - if (shouldRunInBackground) { - // Read subprocess PIDs captured by the pgrep wrapper (non-Windows only) - const backgroundPIDs: number[] = []; - if (!isWindows) { - if (fs.existsSync(tempFilePath)) { - const pgrepLines = fs - .readFileSync(tempFilePath, 'utf8') - .split(EOL) - .filter(Boolean); - for (const line of pgrepLines) { - if (!/^\d+$/.test(line)) { - debugLogger.warn(`pgrep: ${line}`); - continue; - } - const bgPid = Number(line); - if (bgPid !== result.pid) { - backgroundPIDs.push(bgPid); - } - } - } else if (!signal.aborted) { - debugLogger.warn('missing pgrep output'); + break; + default: { + throw new Error('An unhandled ShellOutputEvent was found.'); } } - const bgPidMsg = - backgroundPIDs.length > 0 - ? ` PIDs: ${backgroundPIDs.join(', ')}` - : pid - ? ` PID: ${pid}` - : ''; - const killHint = ' (Use kill to stop)'; - - return { - llmContent: `Background command started.${bgPidMsg}${killHint}`, - returnDisplay: `Background command started.${bgPidMsg}${killHint}`, - }; - } - - let llmContent = ''; - if (result.aborted) { - // Check if it was a timeout or user cancellation - const wasTimeout = - !this.params.is_background && - effectiveTimeout && - combinedSignal.aborted && - !signal.aborted; - - if (wasTimeout) { - llmContent = `Command timed out after ${effectiveTimeout}ms before it could complete.`; - if (result.output.trim()) { - llmContent += ` Below is the output before it timed out:\n${result.output}`; + if (shouldUpdate && updateOutput) { + if (typeof cumulativeOutput === 'string') { + updateOutput(cumulativeOutput); } else { - llmContent += ' There was no output before it timed out.'; - } - } else { - llmContent = - 'Command was cancelled by user before it could complete.'; - if (result.output.trim()) { - llmContent += ` Below is the output before it was cancelled:\n${result.output}`; - } else { - llmContent += ' There was no output before it was cancelled.'; + updateOutput({ + ansiOutput: cumulativeOutput, + totalLines, + totalBytes, + // Only include timeout when user explicitly set it + ...(this.params.timeout != null && { + timeoutMs: this.params.timeout, + }), + }); } + lastUpdateTime = Date.now(); } - } else { - // Create a formatted error string for display, replacing the wrapper command - // with the user-facing command. - const finalError = result.error - ? result.error.message.replace(commandToExecute, this.params.command) - : '(none)'; + }, + combinedSignal, + this.config.getShouldUseNodePtyShell(), + shellExecutionConfig ?? {}, + ); - llmContent = [ - `Command: ${this.params.command}`, - `Directory: ${this.params.directory || '(root)'}`, - `Output: ${result.output || '(empty)'}`, - `Error: ${finalError}`, // Use the cleaned error string. - `Exit Code: ${result.exitCode ?? '(none)'}`, - `Signal: ${result.signal ?? '(none)'}`, - `Process Group PGID: ${result.pid ?? '(none)'}`, - ].join('\n'); - } + if (pid && setPidCallback) { + setPidCallback(pid); + } - let returnDisplayMessage = ''; - if (this.config.getDebugMode()) { - returnDisplayMessage = llmContent; - } else { + const result = await resultPromise; + + let llmContent = ''; + if (result.aborted) { + // Check if it was a timeout or user cancellation + const wasTimeout = + effectiveTimeout && combinedSignal.aborted && !signal.aborted; + + if (wasTimeout) { + llmContent = `Command timed out after ${effectiveTimeout}ms before it could complete.`; if (result.output.trim()) { - returnDisplayMessage = result.output; + llmContent += ` Below is the output before it timed out:\n${result.output}`; } else { - if (result.aborted) { - // Check if it was a timeout or user cancellation - const wasTimeout = - !this.params.is_background && - effectiveTimeout && - combinedSignal.aborted && - !signal.aborted; - - returnDisplayMessage = wasTimeout - ? `Command timed out after ${effectiveTimeout}ms.` - : 'Command cancelled by user.'; - } else if (result.signal) { - returnDisplayMessage = `Command terminated by signal: ${result.signal}`; - } else if (result.error) { - returnDisplayMessage = `Command failed: ${getErrorMessage( - result.error, - )}`; - } else if (result.exitCode !== null && result.exitCode !== 0) { - returnDisplayMessage = `Command exited with code: ${result.exitCode}`; - } - // If output is empty and command succeeded (code 0, no error/signal/abort), - // returnDisplayMessage will remain empty, which is fine. + llmContent += ' There was no output before it timed out.'; + } + } else { + llmContent = 'Command was cancelled by user before it could complete.'; + if (result.output.trim()) { + llmContent += ` Below is the output before it was cancelled:\n${result.output}`; + } else { + llmContent += ' There was no output before it was cancelled.'; } } + } else { + // Create a formatted error string for display, replacing the wrapper command + // with the user-facing command. + const finalError = result.error + ? result.error.message.replace(commandToExecute, this.params.command) + : '(none)'; - // Truncate large output and save full content to a temp file. - if (typeof llmContent === 'string') { - const truncatedResult = await truncateToolOutput( - this.config, - ShellTool.Name, - llmContent, - ); + llmContent = [ + `Command: ${this.params.command}`, + `Directory: ${this.params.directory || '(root)'}`, + `Output: ${result.output || '(empty)'}`, + `Error: ${finalError}`, // Use the cleaned error string. + `Exit Code: ${result.exitCode ?? '(none)'}`, + `Signal: ${result.signal ?? '(none)'}`, + `Process Group PGID: ${result.pid ?? '(none)'}`, + ].join('\n'); + } - if (truncatedResult.outputFile) { - llmContent = truncatedResult.content; - returnDisplayMessage += - (returnDisplayMessage ? '\n' : '') + - `Output too long and was saved to: ${truncatedResult.outputFile}`; + let returnDisplayMessage = ''; + if (this.config.getDebugMode()) { + returnDisplayMessage = llmContent; + } else { + if (result.output.trim()) { + returnDisplayMessage = result.output; + } else { + if (result.aborted) { + // Check if it was a timeout or user cancellation + const wasTimeout = + effectiveTimeout && combinedSignal.aborted && !signal.aborted; + + returnDisplayMessage = wasTimeout + ? `Command timed out after ${effectiveTimeout}ms.` + : 'Command cancelled by user.'; + } else if (result.signal) { + returnDisplayMessage = `Command terminated by signal: ${result.signal}`; + } else if (result.error) { + returnDisplayMessage = `Command failed: ${getErrorMessage( + result.error, + )}`; + } else if (result.exitCode !== null && result.exitCode !== 0) { + returnDisplayMessage = `Command exited with code: ${result.exitCode}`; } - } - - const executionError = result.error - ? { - error: { - message: result.error.message, - type: ToolErrorType.SHELL_EXECUTE_ERROR, - }, - } - : {}; - - return { - llmContent, - returnDisplay: returnDisplayMessage, - ...executionError, - }; - } finally { - if (fs.existsSync(tempFilePath)) { - fs.unlinkSync(tempFilePath); + // If output is empty and command succeeded (code 0, no error/signal/abort), + // returnDisplayMessage will remain empty, which is fine. } } + + // Truncate large output and save full content to a temp file. + if (typeof llmContent === 'string') { + const truncatedResult = await truncateToolOutput( + this.config, + ShellTool.Name, + llmContent, + ); + + if (truncatedResult.outputFile) { + llmContent = truncatedResult.content; + returnDisplayMessage += + (returnDisplayMessage ? '\n' : '') + + `Output too long and was saved to: ${truncatedResult.outputFile}`; + } + } + + const executionError = result.error + ? { + error: { + message: result.error.message, + type: ToolErrorType.SHELL_EXECUTE_ERROR, + }, + } + : {}; + + return { + llmContent, + returnDisplay: returnDisplayMessage, + ...executionError, + }; + } + + /** + * Background-execution path: spawn the command into a managed registry + * entry instead of detaching with `&`. Output streams to a per-shell file + * the agent can `Read`; cancellation flows through the entry's + * AbortController; the registry's terminal status is set when the process + * exits. Returns immediately so the agent's turn isn't blocked. + */ + private async executeBackground( + signal: AbortSignal, + shellExecutionConfig?: ShellExecutionConfig, + ): Promise { + const strippedCommand = stripShellWrapper(this.params.command); + const processedCommand = this.addCoAuthorToGitCommit(strippedCommand); + const cwd = this.params.directory || this.config.getTargetDir(); + + // Per-session output dir under the project. Path layout aligns with the + // direction sketched in #3471 review (`/tasks//`) + // so future task kinds (subagent transcripts, monitor logs) sit alongside. + const outputDir = path.join( + this.config.storage.getProjectDir(), + 'tasks', + this.config.getSessionId(), + ); + fs.mkdirSync(outputDir, { recursive: true }); + + const shellId = `bg_${crypto.randomBytes(4).toString('hex')}`; + const outputPath = path.join(outputDir, `shell-${shellId}.output`); + + // Bridge external signal (tool-framework abort, e.g. Ctrl+C) into the + // entry's AC so a single source of truth governs cancellation. + const entryAc = new AbortController(); + if (signal.aborted) { + entryAc.abort(); + } else { + signal.addEventListener('abort', () => entryAc.abort(), { once: true }); + } + + const outputStream = fs.createWriteStream(outputPath, { flags: 'w' }); + + const startTime = Date.now(); + const entry: BackgroundShellEntry = { + shellId, + command: processedCommand, + cwd, + status: 'running', + startTime, + outputPath, + abortController: entryAc, + }; + + const { result: resultPromise, pid } = await ShellExecutionService.execute( + processedCommand, + cwd, + (event: ShellOutputEvent) => { + if (event.type === 'data' && typeof event.chunk === 'string') { + outputStream.write(event.chunk); + } + // ANSI array chunks and binary streams are not written to the output + // file: agents read the file as plain text and binary spam would be + // unhelpful. + }, + entryAc.signal, + false, + shellExecutionConfig ?? {}, + ); + + if (pid !== undefined) entry.pid = pid; + const registry = this.config.getBackgroundShellRegistry(); + registry.register(entry); + + // Settle in the background — do NOT await here, the agent should be + // unblocked immediately. + void resultPromise.then( + (result) => { + outputStream.end(); + const endTime = Date.now(); + if (entryAc.signal.aborted) { + if (registry.get(shellId)?.status === 'running') { + registry.cancel(shellId, endTime); + } + } else if (result.error) { + registry.fail(shellId, result.error.message, endTime); + } else { + registry.complete(shellId, result.exitCode ?? 0, endTime); + } + }, + (err) => { + outputStream.end(); + registry.fail(shellId, getErrorMessage(err), Date.now()); + }, + ); + + const pidLine = pid !== undefined ? `pid: ${pid}\n` : ''; + return { + llmContent: + `Background shell started.\n` + + `id: ${shellId}\n` + + pidLine + + `output file: ${outputPath}\n` + + `Use the /bashes command to list and inspect background shells, or Read the output file directly.`, + returnDisplay: `Background shell ${shellId} started${pid !== undefined ? ` (pid ${pid})` : ''}.`, + }; } private addCoAuthorToGitCommit(command: string): string {