Skip to content

Read timeout for command_execute #134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 27, 2025
Merged

Read timeout for command_execute #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
684fd7f
fix: added timeout_ms to execute_command
serg33v May 22, 2025
37206bc
feat: read output timeout
serg33v May 22, 2025
5365636
fix: fixed racing condition
serg33v May 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions setup-claude-server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,16 @@ export default async function setup() {
const setupStep = addSetupStep('main_setup');
const debugMode = isDebugMode();

// Print ASCII art for DESKTOP COMMANDER
console.log('\n');
console.log('██████╗ ███████╗███████╗██╗ ██╗████████╗ ██████╗ ██████╗ ██████╗ ██████╗ ███╗ ███╗███╗ ███╗ █████╗ ███╗ ██╗██████╗ ███████╗██████╗ ');
console.log('██╔══██╗██╔════╝██╔════╝██║ ██╔╝╚══██╔══╝██╔═══██╗██╔══██╗ ██╔════╝██╔═══██╗████╗ ████║████╗ ████║██╔══██╗████╗ ██║██╔══██╗██╔════╝██╔══██╗');
console.log('██║ ██║█████╗ ███████╗█████╔╝ ██║ ██║ ██║██████╔╝ ██║ ██║ ██║██╔████╔██║██╔████╔██║███████║██╔██╗ ██║██║ ██║█████╗ ██████╔╝');
console.log('██║ ██║██╔══╝ ╚════██║██╔═██╗ ██║ ██║ ██║██╔═══╝ ██║ ██║ ██║██║╚██╔╝██║██║╚██╔╝██║██╔══██║██║╚██╗██║██║ ██║██╔══╝ ██╔══██╗');
console.log('██████╔╝███████╗███████║██║ ██╗ ██║ ╚██████╔╝██║ ╚██████╗╚██████╔╝██║ ╚═╝ ██║██║ ╚═╝ ██║██║ ██║██║ ╚████║██████╔╝███████╗██║ ██║');
console.log('╚═════╝ ╚══════╝╚══════╝╚═╝ ╚═╝ ╚═╝ ╚═════╝ ╚═╝ ╚═════╝ ╚═════╝ ╚═╝ ╚═╝╚═╝ ╚═╝╚═╝ ╚═╝╚═╝ ╚═══╝╚═════╝ ╚══════╝╚═╝ ╚═╝');
console.log('\n');

if (debugMode) {
logToFile('Debug mode enabled. Will configure with Node.js inspector options.');
await trackEvent('npx_setup_debug_mode', { enabled: true });
Expand Down Expand Up @@ -726,6 +736,8 @@ export default async function setup() {
total_time_ms: Date.now() - setupStartTime
});



return true;
} catch (error) {
updateSetupStep(setupStep, 'failed', error);
Expand Down
1 change: 1 addition & 0 deletions src/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ server.setRequestHandler(ListToolsRequestSchema, async () => {
name: "read_output",
description: `
Read new output from a running terminal session.
Set timeout_ms for long running commands.

${CMD_PREFIX_DESCRIPTION}`,
inputSchema: zodToJsonSchema(ReadOutputArgsSchema),
Expand Down
9 changes: 9 additions & 0 deletions src/terminal-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,15 @@ export class TerminalManager {
return null;
}

/**
* Get a session by PID
* @param pid Process ID
* @returns The session or undefined if not found
*/
getSession(pid: number): TerminalSession | undefined {
return this.sessions.get(pid);
}

forceTerminate(pid: number): boolean {
const session = this.sessions.get(pid);
if (!session) {
Expand Down
88 changes: 77 additions & 11 deletions src/tools/execute.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,21 +62,87 @@ export async function executeCommand(args: unknown): Promise<ServerResult> {
}

export async function readOutput(args: unknown): Promise<ServerResult> {
const parsed = ReadOutputArgsSchema.safeParse(args);
if (!parsed.success) {
return {
content: [{ type: "text", text: `Error: Invalid arguments for read_output: ${parsed.error}` }],
isError: true,
};
}
const parsed = ReadOutputArgsSchema.safeParse(args);
if (!parsed.success) {
return {
content: [{ type: "text", text: `Error: Invalid arguments for read_output: ${parsed.error}` }],
isError: true,
};
}

const { pid, timeout_ms = 5000 } = parsed.data;

// Check if the process exists
const session = terminalManager.getSession(pid);
if (!session) {
return {
content: [{ type: "text", text: `No session found for PID ${pid}` }],
isError: true,
};
}
// Wait for output with timeout
let output = "";
let timeoutReached = false;
try {
// Create a promise that resolves when new output is available or when timeout is reached
const outputPromise: Promise<string> = new Promise<string>((resolve) => {
// Check for initial output
const initialOutput = terminalManager.getNewOutput(pid);
if (initialOutput && initialOutput.length > 0) {
resolve(initialOutput);
return;
}

let resolved = false;
let interval: NodeJS.Timeout | null = null;
let timeout: NodeJS.Timeout | null = null;

const cleanup = () => {
if (interval) {
clearInterval(interval);
interval = null;
}
if (timeout) {
clearTimeout(timeout);
timeout = null;
}
};

const resolveOnce = (value: string, isTimeout = false) => {
if (resolved) return;
resolved = true;
cleanup();
if (isTimeout) timeoutReached = true;
resolve(value);
};

// Setup an interval to poll for output
interval = setInterval(() => {
const newOutput = terminalManager.getNewOutput(pid);
if (newOutput && newOutput.length > 0) {
resolveOnce(newOutput);
}
}, 300); // Check every 300ms

// Set a timeout to stop waiting
timeout = setTimeout(() => {
const finalOutput = terminalManager.getNewOutput(pid) || "";
resolveOnce(finalOutput, true);
}, timeout_ms);
});

output = await outputPromise;
} catch (error) {
return {
content: [{ type: "text", text: `Error reading output: ${error}` }],
isError: true,
};
}
Comment on lines +84 to +140
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Consider optimizing the polling interval.

The implementation is well-structured with proper cleanup and race condition handling. However, the 300ms polling interval might cause unnecessary CPU usage, especially when multiple sessions are being monitored.

Consider making the polling interval configurable or using an exponential backoff strategy.

Additionally, verify that the code handles the case where a session terminates while waiting for output:

🏁 Script executed:

#!/bin/bash
# Description: Check if terminalManager handles terminated sessions gracefully

# Search for session termination handling in terminal-manager
ast-grep --pattern 'getNewOutput($_, $_) {
  $$$
}'

# Look for any session state checks
rg -A 10 "getNewOutput|getSession" src/terminal-manager.ts

Length of output: 698

Optimize polling interval in execute.ts

The hard-coded 300 ms polling interval can add up under load. Consider making it configurable (e.g. via an optional parameter or environment setting) or switching to an exponential backoff strategy to reduce CPU usage when output is sparse.

Note: session termination is already handled by terminalManager.getNewOutput(), which checks both active and completed sessions. No additional checks are needed for cleanup here.

• File: src/tools/execute.ts
• Lines: ~84–140

🤖 Prompt for AI Agents 
In src/tools/execute.ts around lines 84 to 140, the polling interval for
checking new output is hard-coded to 300ms, which can cause unnecessary CPU
usage under load. Modify the code to make the polling interval configurable by
adding an optional parameter or reading from an environment variable.
Alternatively, implement an exponential backoff strategy that increases the
interval between polls when no new output is detected, resetting it when output
arrives. This will optimize resource usage while maintaining responsiveness.


const output = terminalManager.getNewOutput(parsed.data.pid);
return {
content: [{
type: "text",
text: output === null
? `No session found for PID ${parsed.data.pid}`
: output || 'No new output available'
text: output || 'No new output available' + (timeoutReached ? ' (timeout reached)' : '')
}],
};
}
Expand Down
3 changes: 2 additions & 1 deletion src/tools/schemas.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@ export const ListProcessesArgsSchema = z.object({});
// Terminal tools schemas
export const ExecuteCommandArgsSchema = z.object({
command: z.string(),
timeout_ms: z.number().optional(),
timeout_ms: z.number(),
shell: z.string().optional(),
});

export const ReadOutputArgsSchema = z.object({
pid: z.number(),
timeout_ms: z.number().optional(),
});

export const ForceTerminateArgsSchema = z.object({
Expand Down