mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-04-28 03:30:40 +00:00
feat(core): managed background shell pool with /bashes command
Replace shell.ts's `&` fork-and-detach background path with a managed process registry. Background shells now have observable lifecycle, captured output, and explicit cancellation — matching the pattern used by background subagents (#3076). Phase B from #3634 (background task management roadmap). What changes - New `BackgroundShellRegistry` (services/backgroundShellRegistry.ts): per-process entry with status (running / completed / failed / cancelled), AbortController, output file path. State transitions are one-shot (terminal status sticks; late callbacks no-op). Mirrors the lifecycle shape of #3471's BackgroundTaskRegistry so the two can be unified later. - `shell.ts` is_background path rewritten as `executeBackground`: - Spawns the unwrapped command (no '&', no pgrep envelope) - Streams stdout to `<projectDir>/tasks/<sessionId>/shell-<id>.output` (path layout aligns with the direction sketched in #3471 review) - Bridges the external abort signal into the entry's AbortController so a single source of truth governs cancellation - Returns immediately with id + output path; agent's turn isn't blocked - Settles the registry entry asynchronously when ShellExecutionService resolves: complete (clean exit) / fail (error) / cancel (aborted) - Removes ~120 lines of dead bg-specific code from shell.ts: pgrep wrapping, '&' appending, Windows ampersand cleanup, Windows early-return path, bg PID parsing, tempFile cleanup - New `/bashes` slash command: lists registered shells with id, status, runtime, command, output path. Empty state prints a friendly message. What this PR doesn't do - Footer pill / dialog integration — gated on #3488 landing - task_stop / send_message integration — gated on #3471 landing - Auto-backgrounding heuristics for long foreground bash — Phase D Test plan - 11 registry unit tests (state machine + idempotent terminal transitions) - 4 background-path tests in shell.test.ts (spawn no-wrap + complete / fail / cancel settle paths) - 2 /bashes command tests (empty + populated) - Full core suite: 247 files / 6075 passed (existing tests unaffected)
This commit is contained in:
parent
569cfe10fa
commit
c8d7d151a1
9 changed files with 791 additions and 406 deletions
|
|
@ -8,6 +8,7 @@ import type { ICommandLoader } from './types.js';
|
||||||
import type { SlashCommand } from '../ui/commands/types.js';
|
import type { SlashCommand } from '../ui/commands/types.js';
|
||||||
import type { Config } from '@qwen-code/qwen-code-core';
|
import type { Config } from '@qwen-code/qwen-code-core';
|
||||||
import { aboutCommand } from '../ui/commands/aboutCommand.js';
|
import { aboutCommand } from '../ui/commands/aboutCommand.js';
|
||||||
|
import { bashesCommand } from '../ui/commands/bashesCommand.js';
|
||||||
import { agentsCommand } from '../ui/commands/agentsCommand.js';
|
import { agentsCommand } from '../ui/commands/agentsCommand.js';
|
||||||
import { arenaCommand } from '../ui/commands/arenaCommand.js';
|
import { arenaCommand } from '../ui/commands/arenaCommand.js';
|
||||||
import { approvalModeCommand } from '../ui/commands/approvalModeCommand.js';
|
import { approvalModeCommand } from '../ui/commands/approvalModeCommand.js';
|
||||||
|
|
@ -91,6 +92,7 @@ export class BuiltinCommandLoader implements ICommandLoader {
|
||||||
const allDefinitions: Array<SlashCommand | null> = [
|
const allDefinitions: Array<SlashCommand | null> = [
|
||||||
aboutCommand,
|
aboutCommand,
|
||||||
agentsCommand,
|
agentsCommand,
|
||||||
|
bashesCommand,
|
||||||
arenaCommand,
|
arenaCommand,
|
||||||
approvalModeCommand,
|
approvalModeCommand,
|
||||||
authCommand,
|
authCommand,
|
||||||
|
|
|
||||||
94
packages/cli/src/ui/commands/bashesCommand.test.ts
Normal file
94
packages/cli/src/ui/commands/bashesCommand.test.ts
Normal file
|
|
@ -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> = {},
|
||||||
|
): 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<typeof vi.fn>;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
getAll = vi.fn().mockReturnValue([]);
|
||||||
|
context = createMockCommandContext({
|
||||||
|
services: {
|
||||||
|
config: {
|
||||||
|
getBackgroundShellRegistry: () => ({ getAll }),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
} as unknown as Parameters<typeof createMockCommandContext>[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',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
85
packages/cli/src/ui/commands/bashesCommand.ts
Normal file
85
packages/cli/src/ui/commands/bashesCommand.ts
Normal file
|
|
@ -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'),
|
||||||
|
};
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
@ -61,6 +61,7 @@ import { PermissionManager } from '../permissions/permission-manager.js';
|
||||||
import { SubagentManager } from '../subagents/subagent-manager.js';
|
import { SubagentManager } from '../subagents/subagent-manager.js';
|
||||||
import type { SubagentConfig } from '../subagents/types.js';
|
import type { SubagentConfig } from '../subagents/types.js';
|
||||||
import { BackgroundTaskRegistry } from '../agents/background-tasks.js';
|
import { BackgroundTaskRegistry } from '../agents/background-tasks.js';
|
||||||
|
import { BackgroundShellRegistry } from '../services/backgroundShellRegistry.js';
|
||||||
import {
|
import {
|
||||||
DEFAULT_OTLP_ENDPOINT,
|
DEFAULT_OTLP_ENDPOINT,
|
||||||
DEFAULT_TELEMETRY_TARGET,
|
DEFAULT_TELEMETRY_TARGET,
|
||||||
|
|
@ -544,6 +545,7 @@ export class Config {
|
||||||
private promptRegistry!: PromptRegistry;
|
private promptRegistry!: PromptRegistry;
|
||||||
private subagentManager!: SubagentManager;
|
private subagentManager!: SubagentManager;
|
||||||
private readonly backgroundTaskRegistry = new BackgroundTaskRegistry();
|
private readonly backgroundTaskRegistry = new BackgroundTaskRegistry();
|
||||||
|
private readonly backgroundShellRegistry = new BackgroundShellRegistry();
|
||||||
private extensionManager!: ExtensionManager;
|
private extensionManager!: ExtensionManager;
|
||||||
private skillManager: SkillManager | null = null;
|
private skillManager: SkillManager | null = null;
|
||||||
private permissionManager: PermissionManager | null = null;
|
private permissionManager: PermissionManager | null = null;
|
||||||
|
|
@ -2467,6 +2469,10 @@ export class Config {
|
||||||
return this.backgroundTaskRegistry;
|
return this.backgroundTaskRegistry;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
getBackgroundShellRegistry(): BackgroundShellRegistry {
|
||||||
|
return this.backgroundShellRegistry;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Whether interactive permission prompts should be auto-denied.
|
* Whether interactive permission prompts should be auto-denied.
|
||||||
* True for background agents that have no UI to show prompts.
|
* True for background agents that have no UI to show prompts.
|
||||||
|
|
|
||||||
|
|
@ -147,6 +147,7 @@ export * from './services/sessionService.js';
|
||||||
export * from './services/sessionTitle.js';
|
export * from './services/sessionTitle.js';
|
||||||
export { stripTerminalControlSequences } from './utils/terminalSafe.js';
|
export { stripTerminalControlSequences } from './utils/terminalSafe.js';
|
||||||
export * from './services/shellExecutionService.js';
|
export * from './services/shellExecutionService.js';
|
||||||
|
export * from './services/backgroundShellRegistry.js';
|
||||||
export * from './utils/bareMode.js';
|
export * from './utils/bareMode.js';
|
||||||
|
|
||||||
// ============================================================================
|
// ============================================================================
|
||||||
|
|
|
||||||
132
packages/core/src/services/backgroundShellRegistry.test.ts
Normal file
132
packages/core/src/services/backgroundShellRegistry.test.ts
Normal file
|
|
@ -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> = {},
|
||||||
|
): 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();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
88
packages/core/src/services/backgroundShellRegistry.ts
Normal file
88
packages/core/src/services/backgroundShellRegistry.ts
Normal file
|
|
@ -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<string, BackgroundShellEntry>();
|
||||||
|
|
||||||
|
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();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -31,8 +31,6 @@ import {
|
||||||
} from '../services/shellExecutionService.js';
|
} from '../services/shellExecutionService.js';
|
||||||
import * as fs from 'node:fs';
|
import * as fs from 'node:fs';
|
||||||
import * as os from 'node:os';
|
import * as os from 'node:os';
|
||||||
import { EOL } from 'node:os';
|
|
||||||
import * as path from 'node:path';
|
|
||||||
import * as crypto from 'node:crypto';
|
import * as crypto from 'node:crypto';
|
||||||
import { ToolErrorType } from './tool-error.js';
|
import { ToolErrorType } from './tool-error.js';
|
||||||
import { OUTPUT_UPDATE_INTERVAL_MS } from './shell.js';
|
import { OUTPUT_UPDATE_INTERVAL_MS } from './shell.js';
|
||||||
|
|
@ -55,12 +53,14 @@ describe('ShellTool', () => {
|
||||||
getPermissionsDeny: vi.fn().mockReturnValue([]),
|
getPermissionsDeny: vi.fn().mockReturnValue([]),
|
||||||
getDebugMode: vi.fn().mockReturnValue(false),
|
getDebugMode: vi.fn().mockReturnValue(false),
|
||||||
getTargetDir: vi.fn().mockReturnValue('/test/dir'),
|
getTargetDir: vi.fn().mockReturnValue('/test/dir'),
|
||||||
|
getSessionId: vi.fn().mockReturnValue('test-session'),
|
||||||
getWorkspaceContext: vi
|
getWorkspaceContext: vi
|
||||||
.fn()
|
.fn()
|
||||||
.mockReturnValue(createMockWorkspaceContext('/test/dir')),
|
.mockReturnValue(createMockWorkspaceContext('/test/dir')),
|
||||||
storage: {
|
storage: {
|
||||||
getUserSkillsDirs: vi.fn().mockReturnValue(['/test/dir/.qwen/skills']),
|
getUserSkillsDirs: vi.fn().mockReturnValue(['/test/dir/.qwen/skills']),
|
||||||
getProjectTempDir: vi.fn().mockReturnValue('/tmp/qwen-temp'),
|
getProjectTempDir: vi.fn().mockReturnValue('/tmp/qwen-temp'),
|
||||||
|
getProjectDir: vi.fn().mockReturnValue('/test/proj'),
|
||||||
},
|
},
|
||||||
getTruncateToolOutputThreshold: vi.fn().mockReturnValue(0),
|
getTruncateToolOutputThreshold: vi.fn().mockReturnValue(0),
|
||||||
getTruncateToolOutputLines: vi.fn().mockReturnValue(0),
|
getTruncateToolOutputLines: vi.fn().mockReturnValue(0),
|
||||||
|
|
@ -72,8 +72,23 @@ describe('ShellTool', () => {
|
||||||
email: 'qwen-coder@alibabacloud.com',
|
email: 'qwen-coder@alibabacloud.com',
|
||||||
}),
|
}),
|
||||||
getShouldUseNodePtyShell: vi.fn().mockReturnValue(false),
|
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;
|
} 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);
|
shellTool = new ShellTool(mockConfig);
|
||||||
|
|
||||||
vi.mocked(os.platform).mockReturnValue('linux');
|
vi.mocked(os.platform).mockReturnValue('linux');
|
||||||
|
|
@ -271,81 +286,129 @@ describe('ShellTool', () => {
|
||||||
resolveExecutionPromise(fullResult);
|
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({
|
const invocation = shellTool.build({
|
||||||
command: 'my-command',
|
command: 'npm start',
|
||||||
is_background: true,
|
is_background: true,
|
||||||
});
|
});
|
||||||
const promise = invocation.execute(mockAbortSignal);
|
|
||||||
resolveShellExecution({ pid: 54321 });
|
|
||||||
|
|
||||||
vi.mocked(fs.existsSync).mockReturnValue(true);
|
const result = await invocation.execute(mockAbortSignal);
|
||||||
vi.mocked(fs.readFileSync).mockReturnValue(`54321${EOL}54322${EOL}`); // Service PID and background PID
|
|
||||||
|
|
||||||
const result = await promise;
|
// Spawn happens with the unwrapped command — no '&', no pgrep envelope.
|
||||||
|
|
||||||
const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp');
|
|
||||||
const wrappedCommand = `{ my-command & }; __code=$?; pgrep -g 0 >${tmpFile} 2>&1; exit $__code;`;
|
|
||||||
expect(mockShellExecutionService).toHaveBeenCalledWith(
|
expect(mockShellExecutionService).toHaveBeenCalledWith(
|
||||||
wrappedCommand,
|
'npm start',
|
||||||
'/test/dir',
|
'/test/dir',
|
||||||
expect.any(Function),
|
expect.any(Function),
|
||||||
expect.any(AbortSignal),
|
expect.any(AbortSignal),
|
||||||
false,
|
false,
|
||||||
{},
|
{},
|
||||||
);
|
);
|
||||||
expect(result.llmContent).toContain('PIDs: 54322');
|
// Entry registered with the spawn pid.
|
||||||
expect(vi.mocked(fs.unlinkSync)).toHaveBeenCalledWith(tmpFile);
|
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({
|
const invocation = shellTool.build({
|
||||||
command: 'npm start',
|
command: 'true',
|
||||||
is_background: true,
|
is_background: true,
|
||||||
});
|
});
|
||||||
const promise = invocation.execute(mockAbortSignal);
|
await invocation.execute(mockAbortSignal);
|
||||||
resolveShellExecution({ pid: 54321 });
|
const entry = (registry.register as Mock).mock.calls[0][0];
|
||||||
|
|
||||||
vi.mocked(fs.existsSync).mockReturnValue(true);
|
resolveExecutionPromise({
|
||||||
vi.mocked(fs.readFileSync).mockReturnValue('54321\n54322\n');
|
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;
|
expect(registry.complete).toHaveBeenCalledWith(
|
||||||
|
entry.shellId,
|
||||||
const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp');
|
0,
|
||||||
const wrappedCommand = `{ npm start & }; __code=$?; pgrep -g 0 >${tmpFile} 2>&1; exit $__code;`;
|
expect.any(Number),
|
||||||
expect(mockShellExecutionService).toHaveBeenCalledWith(
|
|
||||||
wrappedCommand,
|
|
||||||
expect.any(String),
|
|
||||||
expect.any(Function),
|
|
||||||
expect.any(AbortSignal),
|
|
||||||
false,
|
|
||||||
{},
|
|
||||||
);
|
);
|
||||||
|
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({
|
const invocation = shellTool.build({
|
||||||
command: 'npm start &',
|
command: 'no-such-command',
|
||||||
is_background: true,
|
is_background: true,
|
||||||
});
|
});
|
||||||
const promise = invocation.execute(mockAbortSignal);
|
await invocation.execute(mockAbortSignal);
|
||||||
resolveShellExecution({ pid: 54321 });
|
const entry = (registry.register as Mock).mock.calls[0][0];
|
||||||
|
|
||||||
vi.mocked(fs.existsSync).mockReturnValue(true);
|
resolveExecutionPromise({
|
||||||
vi.mocked(fs.readFileSync).mockReturnValue('54321\n54322\n');
|
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;
|
expect(registry.fail).toHaveBeenCalledWith(
|
||||||
|
entry.shellId,
|
||||||
const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp');
|
'spawn ENOENT',
|
||||||
const wrappedCommand = `{ npm start & }; __code=$?; pgrep -g 0 >${tmpFile} 2>&1; exit $__code;`;
|
expect.any(Number),
|
||||||
expect(mockShellExecutionService).toHaveBeenCalledWith(
|
|
||||||
wrappedCommand,
|
|
||||||
expect.any(String),
|
|
||||||
expect.any(Function),
|
|
||||||
expect.any(AbortSignal),
|
|
||||||
false,
|
|
||||||
{},
|
|
||||||
);
|
);
|
||||||
|
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 () => {
|
it('should not add ampersand when is_background is false', async () => {
|
||||||
|
|
@ -479,23 +542,6 @@ describe('ShellTool', () => {
|
||||||
).toThrow('Directory must be an absolute path.');
|
).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`', () => {
|
describe('Streaming to `updateOutput`', () => {
|
||||||
let updateOutputMock: Mock;
|
let updateOutputMock: Mock;
|
||||||
beforeEach(() => {
|
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', () => {
|
describe('timeout parameter', () => {
|
||||||
it('should validate timeout parameter correctly', async () => {
|
it('should validate timeout parameter correctly', async () => {
|
||||||
// Valid timeout
|
// Valid timeout
|
||||||
|
|
@ -1177,40 +1186,6 @@ describe('ShellTool', () => {
|
||||||
expect(calledSignal).not.toBe(mockAbortSignal);
|
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 () => {
|
it('should handle timeout vs user cancellation correctly', async () => {
|
||||||
const userAbortController = new AbortController();
|
const userAbortController = new AbortController();
|
||||||
const invocation = shellTool.build({
|
const invocation = shellTool.build({
|
||||||
|
|
|
||||||
|
|
@ -6,7 +6,7 @@
|
||||||
|
|
||||||
import fs from 'node:fs';
|
import fs from 'node:fs';
|
||||||
import path from 'node:path';
|
import path from 'node:path';
|
||||||
import os, { EOL } from 'node:os';
|
import os from 'node:os';
|
||||||
import crypto from 'node:crypto';
|
import crypto from 'node:crypto';
|
||||||
import type { Config } from '../config/config.js';
|
import type { Config } from '../config/config.js';
|
||||||
import { ToolNames, ToolDisplayNames } from './tool-names.js';
|
import { ToolNames, ToolDisplayNames } from './tool-names.js';
|
||||||
|
|
@ -29,6 +29,7 @@ import type {
|
||||||
ShellOutputEvent,
|
ShellOutputEvent,
|
||||||
} from '../services/shellExecutionService.js';
|
} from '../services/shellExecutionService.js';
|
||||||
import { ShellExecutionService } from '../services/shellExecutionService.js';
|
import { ShellExecutionService } from '../services/shellExecutionService.js';
|
||||||
|
import type { BackgroundShellEntry } from '../services/backgroundShellRegistry.js';
|
||||||
import { formatMemoryUsage } from '../utils/formatters.js';
|
import { formatMemoryUsage } from '../utils/formatters.js';
|
||||||
import type { AnsiOutput } from '../utils/terminalSerializer.js';
|
import type { AnsiOutput } from '../utils/terminalSerializer.js';
|
||||||
import { isSubpaths } from '../utils/paths.js';
|
import { isSubpaths } from '../utils/paths.js';
|
||||||
|
|
@ -208,9 +209,12 @@ export class ShellToolInvocation extends BaseToolInvocation<
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
const effectiveTimeout = this.params.is_background
|
if (this.params.is_background) {
|
||||||
? undefined
|
return this.executeBackground(signal, shellExecutionConfig);
|
||||||
: (this.params.timeout ?? DEFAULT_FOREGROUND_TIMEOUT_MS);
|
}
|
||||||
|
|
||||||
|
const effectiveTimeout =
|
||||||
|
this.params.timeout ?? DEFAULT_FOREGROUND_TIMEOUT_MS;
|
||||||
|
|
||||||
// Create combined signal with timeout for foreground execution
|
// Create combined signal with timeout for foreground execution
|
||||||
let combinedSignal = signal;
|
let combinedSignal = signal;
|
||||||
|
|
@ -219,48 +223,9 @@ export class ShellToolInvocation extends BaseToolInvocation<
|
||||||
combinedSignal = AbortSignal.any([signal, timeoutSignal]);
|
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);
|
|
||||||
|
|
||||||
try {
|
|
||||||
// Add co-author to git commit commands
|
// Add co-author to git commit commands
|
||||||
const processedCommand = this.addCoAuthorToGitCommit(strippedCommand);
|
const processedCommand = this.addCoAuthorToGitCommit(strippedCommand);
|
||||||
|
const commandToExecute = processedCommand;
|
||||||
const shouldRunInBackground = this.params.is_background;
|
|
||||||
let finalCommand = processedCommand;
|
|
||||||
|
|
||||||
// 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();
|
const cwd = this.params.directory || this.config.getTargetDir();
|
||||||
|
|
||||||
let cumulativeOutput: string | AnsiOutput = '';
|
let cumulativeOutput: string | AnsiOutput = '';
|
||||||
|
|
@ -269,8 +234,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
|
||||||
let totalLines = 0;
|
let totalLines = 0;
|
||||||
let totalBytes = 0;
|
let totalBytes = 0;
|
||||||
|
|
||||||
const { result: resultPromise, pid } =
|
const { result: resultPromise, pid } = await ShellExecutionService.execute(
|
||||||
await ShellExecutionService.execute(
|
|
||||||
commandToExecute,
|
commandToExecute,
|
||||||
cwd,
|
cwd,
|
||||||
(event: ShellOutputEvent) => {
|
(event: ShellOutputEvent) => {
|
||||||
|
|
@ -288,8 +252,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
|
||||||
(sum, line) =>
|
(sum, line) =>
|
||||||
sum +
|
sum +
|
||||||
line.reduce(
|
line.reduce(
|
||||||
(ls, token) =>
|
(ls, token) => ls + Buffer.byteLength(token.text, 'utf-8'),
|
||||||
ls + Buffer.byteLength(token.text, 'utf-8'),
|
|
||||||
0,
|
0,
|
||||||
),
|
),
|
||||||
0,
|
0,
|
||||||
|
|
@ -299,8 +262,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
|
||||||
break;
|
break;
|
||||||
case 'binary_detected':
|
case 'binary_detected':
|
||||||
isBinaryStream = true;
|
isBinaryStream = true;
|
||||||
cumulativeOutput =
|
cumulativeOutput = '[Binary output detected. Halting stream...]';
|
||||||
'[Binary output detected. Halting stream...]';
|
|
||||||
shouldUpdate = true;
|
shouldUpdate = true;
|
||||||
break;
|
break;
|
||||||
case 'binary_progress':
|
case 'binary_progress':
|
||||||
|
|
@ -335,9 +297,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
combinedSignal,
|
combinedSignal,
|
||||||
shouldRunInBackground
|
this.config.getShouldUseNodePtyShell(),
|
||||||
? false
|
|
||||||
: this.config.getShouldUseNodePtyShell(),
|
|
||||||
shellExecutionConfig ?? {},
|
shellExecutionConfig ?? {},
|
||||||
);
|
);
|
||||||
|
|
||||||
|
|
@ -345,66 +305,13 @@ export class ShellToolInvocation extends BaseToolInvocation<
|
||||||
setPidCallback(pid);
|
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 <pid> to stop)';
|
|
||||||
|
|
||||||
return {
|
|
||||||
llmContent: `Background command started.${pidMsg}${killHint}`,
|
|
||||||
returnDisplay: `Background command started.${pidMsg}${killHint}`,
|
|
||||||
};
|
|
||||||
}
|
|
||||||
|
|
||||||
const result = await resultPromise;
|
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');
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
const bgPidMsg =
|
|
||||||
backgroundPIDs.length > 0
|
|
||||||
? ` PIDs: ${backgroundPIDs.join(', ')}`
|
|
||||||
: pid
|
|
||||||
? ` PID: ${pid}`
|
|
||||||
: '';
|
|
||||||
const killHint = ' (Use kill <pid> to stop)';
|
|
||||||
|
|
||||||
return {
|
|
||||||
llmContent: `Background command started.${bgPidMsg}${killHint}`,
|
|
||||||
returnDisplay: `Background command started.${bgPidMsg}${killHint}`,
|
|
||||||
};
|
|
||||||
}
|
|
||||||
|
|
||||||
let llmContent = '';
|
let llmContent = '';
|
||||||
if (result.aborted) {
|
if (result.aborted) {
|
||||||
// Check if it was a timeout or user cancellation
|
// Check if it was a timeout or user cancellation
|
||||||
const wasTimeout =
|
const wasTimeout =
|
||||||
!this.params.is_background &&
|
effectiveTimeout && combinedSignal.aborted && !signal.aborted;
|
||||||
effectiveTimeout &&
|
|
||||||
combinedSignal.aborted &&
|
|
||||||
!signal.aborted;
|
|
||||||
|
|
||||||
if (wasTimeout) {
|
if (wasTimeout) {
|
||||||
llmContent = `Command timed out after ${effectiveTimeout}ms before it could complete.`;
|
llmContent = `Command timed out after ${effectiveTimeout}ms before it could complete.`;
|
||||||
|
|
@ -414,8 +321,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
|
||||||
llmContent += ' There was no output before it timed out.';
|
llmContent += ' There was no output before it timed out.';
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
llmContent =
|
llmContent = 'Command was cancelled by user before it could complete.';
|
||||||
'Command was cancelled by user before it could complete.';
|
|
||||||
if (result.output.trim()) {
|
if (result.output.trim()) {
|
||||||
llmContent += ` Below is the output before it was cancelled:\n${result.output}`;
|
llmContent += ` Below is the output before it was cancelled:\n${result.output}`;
|
||||||
} else {
|
} else {
|
||||||
|
|
@ -450,10 +356,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
|
||||||
if (result.aborted) {
|
if (result.aborted) {
|
||||||
// Check if it was a timeout or user cancellation
|
// Check if it was a timeout or user cancellation
|
||||||
const wasTimeout =
|
const wasTimeout =
|
||||||
!this.params.is_background &&
|
effectiveTimeout && combinedSignal.aborted && !signal.aborted;
|
||||||
effectiveTimeout &&
|
|
||||||
combinedSignal.aborted &&
|
|
||||||
!signal.aborted;
|
|
||||||
|
|
||||||
returnDisplayMessage = wasTimeout
|
returnDisplayMessage = wasTimeout
|
||||||
? `Command timed out after ${effectiveTimeout}ms.`
|
? `Command timed out after ${effectiveTimeout}ms.`
|
||||||
|
|
@ -502,11 +405,110 @@ export class ShellToolInvocation extends BaseToolInvocation<
|
||||||
returnDisplay: returnDisplayMessage,
|
returnDisplay: returnDisplayMessage,
|
||||||
...executionError,
|
...executionError,
|
||||||
};
|
};
|
||||||
} finally {
|
|
||||||
if (fs.existsSync(tempFilePath)) {
|
|
||||||
fs.unlinkSync(tempFilePath);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 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<ToolResult> {
|
||||||
|
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 (`<projectDir>/tasks/<sessionId>/`)
|
||||||
|
// 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 {
|
private addCoAuthorToGitCommit(command: string): string {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue