From e95e568ef4a3b4bc8a3f713761868c90453e766e Mon Sep 17 00:00:00 2001 From: katereznykova Date: Wed, 29 Oct 2025 14:02:01 +0000 Subject: [PATCH 1/3] Add timeouts to file and git operations to prevent indefinite hangs --- .changeset/fix-file-operations-timeout.md | 6 +++ .../src/services/file-service.ts | 26 +++++------ .../src/services/git-service.ts | 8 ++-- .../src/services/session-manager.ts | 9 ++-- .../tests/services/file-service.test.ts | 44 ++++++++++++------- .../tests/services/git-service.test.ts | 8 ++-- 6 files changed, 58 insertions(+), 43 deletions(-) create mode 100644 .changeset/fix-file-operations-timeout.md diff --git a/.changeset/fix-file-operations-timeout.md b/.changeset/fix-file-operations-timeout.md new file mode 100644 index 00000000..b46dd108 --- /dev/null +++ b/.changeset/fix-file-operations-timeout.md @@ -0,0 +1,6 @@ +--- +"@cloudflare/sandbox": patch +"@repo/sandbox-container": patch +--- + +Fix indefinite hangs in file operations by adding timeouts \ No newline at end of file diff --git a/packages/sandbox-container/src/services/file-service.ts b/packages/sandbox-container/src/services/file-service.ts index fef3c41f..542d25ff 100644 --- a/packages/sandbox-container/src/services/file-service.ts +++ b/packages/sandbox-container/src/services/file-service.ts @@ -98,7 +98,7 @@ export class FileService implements FileSystemOperations { // 3. Get file size using stat const escapedPath = this.escapePath(path); const statCommand = `stat -c '%s' ${escapedPath} 2>/dev/null`; - const statResult = await this.sessionManager.executeInSession(sessionId, statCommand); + const statResult = await this.sessionManager.executeInSession(sessionId, statCommand, { timeoutMs: 5000 }); if (!statResult.success) { return { @@ -134,7 +134,7 @@ export class FileService implements FileSystemOperations { // 4. Detect MIME type using file command const mimeCommand = `file --mime-type -b ${escapedPath}`; - const mimeResult = await this.sessionManager.executeInSession(sessionId, mimeCommand); + const mimeResult = await this.sessionManager.executeInSession(sessionId, mimeCommand, { timeoutMs: 5000 }); if (!mimeResult.success) { return { @@ -183,7 +183,7 @@ export class FileService implements FileSystemOperations { if (isBinary) { // Binary files: read as base64, return as-is (DO NOT decode) const base64Command = `base64 -w 0 < ${escapedPath}`; - const base64Result = await this.sessionManager.executeInSession(sessionId, base64Command); + const base64Result = await this.sessionManager.executeInSession(sessionId, base64Command, { timeoutMs: 30000 }); if (!base64Result.success) { return { @@ -220,7 +220,7 @@ export class FileService implements FileSystemOperations { } else { // Text files: read normally const catCommand = `cat ${escapedPath}`; - const catResult = await this.sessionManager.executeInSession(sessionId, catCommand); + const catResult = await this.sessionManager.executeInSession(sessionId, catCommand, { timeoutMs: 30000 }); if (!catResult.success) { return { @@ -309,7 +309,7 @@ export class FileService implements FileSystemOperations { const base64Content = Buffer.from(content, 'utf-8').toString('base64'); const command = `echo '${base64Content}' | base64 -d > ${escapedPath}`; - const execResult = await this.sessionManager.executeInSession(sessionId, command); + const execResult = await this.sessionManager.executeInSession(sessionId, command, { timeoutMs: 30000 }); if (!execResult.success) { return execResult as ServiceResult; @@ -412,7 +412,7 @@ export class FileService implements FileSystemOperations { const escapedPath = this.escapePath(path); const command = `rm ${escapedPath}`; - const execResult = await this.sessionManager.executeInSession(sessionId, command); + const execResult = await this.sessionManager.executeInSession(sessionId, command, { timeoutMs: 5000 }); if (!execResult.success) { return execResult as ServiceResult; @@ -501,7 +501,7 @@ export class FileService implements FileSystemOperations { const escapedNewPath = this.escapePath(newPath); const command = `mv ${escapedOldPath} ${escapedNewPath}`; - const execResult = await this.sessionManager.executeInSession(sessionId, command); + const execResult = await this.sessionManager.executeInSession(sessionId, command, { timeoutMs: 10000 }); if (!execResult.success) { return execResult as ServiceResult; @@ -586,7 +586,7 @@ export class FileService implements FileSystemOperations { const escapedDest = this.escapePath(destinationPath); const command = `mv ${escapedSource} ${escapedDest}`; - const execResult = await this.sessionManager.executeInSession(sessionId, command); + const execResult = await this.sessionManager.executeInSession(sessionId, command, { timeoutMs: 30000 }); if (!execResult.success) { return execResult as ServiceResult; @@ -656,7 +656,7 @@ export class FileService implements FileSystemOperations { command += ` ${escapedPath}`; // 4. Create directory using SessionManager - const execResult = await this.sessionManager.executeInSession(sessionId, command); + const execResult = await this.sessionManager.executeInSession(sessionId, command, { timeoutMs: 5000 }); if (!execResult.success) { return execResult as ServiceResult; @@ -718,7 +718,7 @@ export class FileService implements FileSystemOperations { const escapedPath = this.escapePath(path); const command = `test -e ${escapedPath}`; - const execResult = await this.sessionManager.executeInSession(sessionId, command); + const execResult = await this.sessionManager.executeInSession(sessionId, command, { timeoutMs: 5000 }); if (!execResult.success) { // If execution fails, treat as non-existent @@ -799,7 +799,7 @@ export class FileService implements FileSystemOperations { const command = `stat ${statCmd.args[0]} ${statCmd.args[1]} ${escapedPath}`; // 5. Get file stats using SessionManager - const execResult = await this.sessionManager.executeInSession(sessionId, command); + const execResult = await this.sessionManager.executeInSession(sessionId, command, { timeoutMs: 5000 }); if (!execResult.success) { return execResult as ServiceResult; @@ -968,7 +968,7 @@ export class FileService implements FileSystemOperations { // Skip the base directory itself and format output findCommand += ` -not -path ${escapedPath} -printf '%p\\t%y\\t%s\\t%TY-%Tm-%TdT%TH:%TM:%TS\\t%m\\n'`; - const execResult = await this.sessionManager.executeInSession(sessionId, findCommand); + const execResult = await this.sessionManager.executeInSession(sessionId, findCommand, { timeoutMs: 30000 }); if (!execResult.success) { return { @@ -1165,7 +1165,7 @@ export class FileService implements FileSystemOperations { command = `dd if=${escapedPath} bs=${chunkSize} skip=${skip} count=${count} 2>/dev/null`; } - const execResult = await this.sessionManager.executeInSession(sessionId, command); + const execResult = await this.sessionManager.executeInSession(sessionId, command, { timeoutMs: 30000 }); if (!execResult.success) { const errorEvent = { diff --git a/packages/sandbox-container/src/services/git-service.ts b/packages/sandbox-container/src/services/git-service.ts index 93553352..82592d5c 100644 --- a/packages/sandbox-container/src/services/git-service.ts +++ b/packages/sandbox-container/src/services/git-service.ts @@ -125,7 +125,7 @@ export class GitService { // explicitly specified or defaulted to the repository's HEAD const branchArgs = this.manager.buildGetCurrentBranchArgs(); const branchCommand = this.buildCommand(branchArgs); - const branchExecResult = await this.sessionManager.executeInSession(sessionId, branchCommand, targetDirectory); + const branchExecResult = await this.sessionManager.executeInSession(sessionId, branchCommand, { cwd: targetDirectory }); if (!branchExecResult.success) { // If we can't get the branch, use fallback but don't fail the entire operation @@ -221,7 +221,7 @@ export class GitService { const command = this.buildCommand(args); // Execute git checkout (via SessionManager) - const execResult = await this.sessionManager.executeInSession(sessionId, command, repoPath); + const execResult = await this.sessionManager.executeInSession(sessionId, command, { cwd: repoPath }); if (!execResult.success) { return execResult as ServiceResult; @@ -301,7 +301,7 @@ export class GitService { const command = this.buildCommand(args); // Execute command (via SessionManager) - const execResult = await this.sessionManager.executeInSession(sessionId, command, repoPath); + const execResult = await this.sessionManager.executeInSession(sessionId, command, { cwd: repoPath }); if (!execResult.success) { return execResult as ServiceResult; @@ -375,7 +375,7 @@ export class GitService { const command = this.buildCommand(args); // Execute command (via SessionManager) - const execResult = await this.sessionManager.executeInSession(sessionId, command, repoPath); + const execResult = await this.sessionManager.executeInSession(sessionId, command, { cwd: repoPath }); if (!execResult.success) { return execResult as ServiceResult; diff --git a/packages/sandbox-container/src/services/session-manager.ts b/packages/sandbox-container/src/services/session-manager.ts index c83bbf96..8f2e4ded 100644 --- a/packages/sandbox-container/src/services/session-manager.ts +++ b/packages/sandbox-container/src/services/session-manager.ts @@ -109,8 +109,7 @@ export class SessionManager { async executeInSession( sessionId: string, command: string, - cwd?: string, - timeoutMs?: number + options?: { cwd?: string; timeoutMs?: number } ): Promise> { try { // Get or create session on demand @@ -120,8 +119,8 @@ export class SessionManager { if (!sessionResult.success && (sessionResult.error!.details as InternalErrorContext)?.originalError === 'Session not found') { sessionResult = await this.createSession({ id: sessionId, - cwd: cwd || '/workspace', - commandTimeoutMs: timeoutMs, // Pass timeout to session + cwd: options?.cwd || '/workspace', + commandTimeoutMs: options?.timeoutMs, // Pass timeout to session }); } @@ -131,7 +130,7 @@ export class SessionManager { const session = sessionResult.data; - const result = await session.exec(command, cwd ? { cwd } : undefined); + const result = await session.exec(command, options?.cwd ? { cwd: options.cwd } : undefined); return { success: true, diff --git a/packages/sandbox-container/tests/services/file-service.test.ts b/packages/sandbox-container/tests/services/file-service.test.ts index 4d1923fd..8098ac19 100644 --- a/packages/sandbox-container/tests/services/file-service.test.ts +++ b/packages/sandbox-container/tests/services/file-service.test.ts @@ -95,13 +95,15 @@ describe('FileService', () => { // Verify MIME type detection command was called expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( 'session-123', - "file --mime-type -b '/tmp/test.txt'" + "file --mime-type -b '/tmp/test.txt'", + { timeoutMs: 5000 } ); // Verify cat was called for text file expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( 'session-123', - "cat '/tmp/test.txt'" + "cat '/tmp/test.txt'", + { timeoutMs: 30000 } ); }); @@ -148,7 +150,8 @@ describe('FileService', () => { // Verify base64 command was called for binary file expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( 'session-123', - "base64 -w 0 < '/tmp/image.png'" + "base64 -w 0 < '/tmp/image.png'", + { timeoutMs: 30000 } ); }); @@ -363,10 +366,11 @@ describe('FileService', () => { expect(result.success).toBe(true); - // Verify SessionManager was called with base64 encoded content (cwd is undefined, so only 2 params) + // Verify SessionManager was called with base64 encoded content expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( 'session-123', - `echo '${base64Content}' | base64 -d > '/tmp/test.txt'` + `echo '${base64Content}' | base64 -d > '/tmp/test.txt'`, + { timeoutMs: 30000 } ); }); @@ -427,12 +431,13 @@ describe('FileService', () => { expect(result.success).toBe(true); - // Verify rm command was called (cwd is undefined, so only 2 params) + // Verify rm command was called // Should be the 4th call expect(mockSessionManager.executeInSession).toHaveBeenNthCalledWith( 4, 'session-123', - "rm '/tmp/test.txt'" + "rm '/tmp/test.txt'", + { timeoutMs: 5000 } ); }); @@ -504,12 +509,13 @@ describe('FileService', () => { expect(mockSecurityService.validatePath).toHaveBeenCalledWith(oldPath); expect(mockSecurityService.validatePath).toHaveBeenCalledWith(newPath); - // Verify mv command was called (cwd is undefined, so only 2 params) + // Verify mv command was called // Should be the 2nd call after exists expect(mockSessionManager.executeInSession).toHaveBeenNthCalledWith( 2, 'session-123', - "mv '/tmp/old.txt' '/tmp/new.txt'" + "mv '/tmp/old.txt' '/tmp/new.txt'", + { timeoutMs: 10000 } ); }); @@ -556,12 +562,13 @@ describe('FileService', () => { expect(result.success).toBe(true); - // Verify mv command was called (cwd is undefined, so only 2 params) + // Verify mv command was called // Should be the 2nd call after exists expect(mockSessionManager.executeInSession).toHaveBeenNthCalledWith( 2, 'session-123', - "mv '/tmp/source.txt' '/tmp/dest.txt'" + "mv '/tmp/source.txt' '/tmp/dest.txt'", + { timeoutMs: 30000 } ); }); @@ -594,10 +601,11 @@ describe('FileService', () => { expect(result.success).toBe(true); - // Verify mkdir command was called (cwd is undefined, so only 2 params) + // Verify mkdir command was called expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( 'session-123', - "mkdir '/tmp/newdir'" + "mkdir '/tmp/newdir'", + { timeoutMs: 5000 } ); }); @@ -613,10 +621,11 @@ describe('FileService', () => { expect(result.success).toBe(true); - // Verify mkdir -p command was called (cwd is undefined, so only 2 params) + // Verify mkdir -p command was called expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( 'session-123', - "mkdir -p '/tmp/nested/dir'" + "mkdir -p '/tmp/nested/dir'", + { timeoutMs: 5000 } ); }); @@ -649,10 +658,11 @@ describe('FileService', () => { expect(result.data).toBe(true); } - // Verify test -e command was called (cwd is undefined, so only 2 params) + // Verify test -e command was called expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( 'session-123', - "test -e '/tmp/test.txt'" + "test -e '/tmp/test.txt'", + { timeoutMs: 5000 } ); }); diff --git a/packages/sandbox-container/tests/services/git-service.test.ts b/packages/sandbox-container/tests/services/git-service.test.ts index 956d8963..156f9ab5 100644 --- a/packages/sandbox-container/tests/services/git-service.test.ts +++ b/packages/sandbox-container/tests/services/git-service.test.ts @@ -97,7 +97,7 @@ describe('GitService', () => { 2, 'default', 'git branch --show-current', - '/workspace/repo' + { cwd: '/workspace/repo' } ); }); @@ -241,7 +241,7 @@ describe('GitService', () => { expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( 'session-123', 'git checkout develop', - '/tmp/repo' + { cwd: '/tmp/repo' } ); }); @@ -298,7 +298,7 @@ describe('GitService', () => { expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( 'session-123', 'git branch --show-current', - '/tmp/repo' + { cwd: '/tmp/repo' } ); }); }); @@ -341,7 +341,7 @@ describe('GitService', () => { expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( 'session-123', 'git branch -a', - '/tmp/repo' + { cwd: '/tmp/repo' } ); }); From 4929d1db12070e401d53261c4398c483529e9cfe Mon Sep 17 00:00:00 2001 From: katereznykova Date: Wed, 29 Oct 2025 14:06:27 +0000 Subject: [PATCH 2/3] fix build --- packages/sandbox-container/src/services/process-service.ts | 3 +-- .../sandbox-container/tests/services/process-service.test.ts | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/sandbox-container/src/services/process-service.ts b/packages/sandbox-container/src/services/process-service.ts index 79cb33d7..67f103e8 100644 --- a/packages/sandbox-container/src/services/process-service.ts +++ b/packages/sandbox-container/src/services/process-service.ts @@ -94,8 +94,7 @@ export class ProcessService { const result = await this.sessionManager.executeInSession( sessionId, command, - options.cwd, - options.timeoutMs + { cwd: options.cwd, timeoutMs: options.timeoutMs } ); if (!result.success) { diff --git a/packages/sandbox-container/tests/services/process-service.test.ts b/packages/sandbox-container/tests/services/process-service.test.ts index d4b64f8e..f4c9cb3b 100644 --- a/packages/sandbox-container/tests/services/process-service.test.ts +++ b/packages/sandbox-container/tests/services/process-service.test.ts @@ -91,8 +91,7 @@ describe('ProcessService', () => { expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( 'default', // sessionId 'echo "hello world"', - '/tmp', // cwd - undefined // timeoutMs (not provided in options) + { cwd: '/tmp', timeoutMs: undefined } // options object ); }); From 19a595f19966e39895d93d4790fbfdc6213c299c Mon Sep 17 00:00:00 2001 From: katereznykova Date: Wed, 29 Oct 2025 14:21:39 +0000 Subject: [PATCH 3/3] Add more timeouts for file and git operations --- .changeset/fix-file-operations-timeout.md | 32 ++++++++++++++++++- .../src/services/file-service.ts | 20 ++++++++++++ .../src/services/git-service.ts | 15 ++++++--- .../src/services/session-manager.ts | 7 +++- packages/sandbox-container/src/session.ts | 16 +++++++--- .../tests/services/git-service.test.ts | 22 +++++++------ 6 files changed, 90 insertions(+), 22 deletions(-) diff --git a/.changeset/fix-file-operations-timeout.md b/.changeset/fix-file-operations-timeout.md index b46dd108..f9a344c8 100644 --- a/.changeset/fix-file-operations-timeout.md +++ b/.changeset/fix-file-operations-timeout.md @@ -3,4 +3,34 @@ "@repo/sandbox-container": patch --- -Fix indefinite hangs in file operations by adding timeouts \ No newline at end of file +Fix indefinite hangs in file and git operations by adding comprehensive timeout support + +**Root Cause (Issue #166):** +File operations like `listFiles()` could hang indefinitely when underlying shell commands (e.g., `find`, `git clone`) took too long or never completed. This was caused by missing timeout parameters in `executeInSession` calls throughout the codebase. + +**Changes:** + +1. **Added per-command timeout support:** + - Extended `Session.ExecOptions` to support `timeoutMs` parameter + - Updated `SessionManager.executeInSession()` to accept options object: `{ cwd?, timeoutMs? }` + - Per-command timeouts now override session-level defaults properly + +2. **Added timeouts to all file operations (13 operations in file-service.ts):** + - Quick operations (5s): `stat`, `exists`, `mkdir`, `delete` + - Medium operations (10s): `rename` + - I/O-heavy operations (30s): `read`, `write`, `move`, `listFiles`, `readFileStream` + - Documented timeout strategy in FileService class documentation + +3. **Added timeouts to all git operations (4 operations in git-service.ts):** + - Clone operations (5min): `cloneRepository` - large repos can take significant time + - Checkout operations (30s): `checkoutBranch` - can be slow with large repos + - Quick operations (10s): `getCurrentBranch`, `listBranches` + +4. **Added timeout to process operations:** + - `ProcessService.executeCommand()` now properly passes timeout through + +**Impact:** +- Fixes issue #166 where `listFiles()` with options would hang indefinitely +- Prevents similar hangs in all file, git, and process operations +- Improves reliability for operations on large directories, slow filesystems, network mounts, or during high I/O load +- Timeout strategy is documented and tiered based on operation complexity \ No newline at end of file diff --git a/packages/sandbox-container/src/services/file-service.ts b/packages/sandbox-container/src/services/file-service.ts index 542d25ff..261b3cec 100644 --- a/packages/sandbox-container/src/services/file-service.ts +++ b/packages/sandbox-container/src/services/file-service.ts @@ -33,6 +33,26 @@ export interface FileSystemOperations { list(path: string, options?: ListFilesOptions, sessionId?: string): Promise>; } +/** + * FileService - Handles file system operations with timeouts + * + * Timeout Strategy: + * All file operations use timeouts to prevent indefinite hangs. Timeouts are tiered based on + * operation complexity and expected execution time: + * + * - Quick operations (5s): stat, exists, mkdir, delete + * These are simple filesystem metadata operations that should complete quickly + * + * - Medium operations (10s): rename + * File renames are typically quick but may take longer on slow filesystems + * + * - I/O-heavy operations (30s): read, write, move, listFiles, readFileStream + * These operations involve actual data transfer or directory traversal and can be slow on: + * - Large files or directories + * - Network filesystems (NFS, CIFS) + * - High I/O load + * - Symlink loops (for listFiles) + */ export class FileService implements FileSystemOperations { private manager: FileManager; diff --git a/packages/sandbox-container/src/services/git-service.ts b/packages/sandbox-container/src/services/git-service.ts index 82592d5c..f667c5cc 100644 --- a/packages/sandbox-container/src/services/git-service.ts +++ b/packages/sandbox-container/src/services/git-service.ts @@ -87,8 +87,9 @@ export class GitService { const command = this.buildCommand(args); // Execute git clone (via SessionManager) + // Clone can take a long time for large repos or slow networks, so use 5 minute timeout const sessionId = options.sessionId || 'default'; - const execResult = await this.sessionManager.executeInSession(sessionId, command); + const execResult = await this.sessionManager.executeInSession(sessionId, command, { timeoutMs: 300000 }); if (!execResult.success) { return execResult as ServiceResult<{ path: string; branch: string }>; @@ -125,7 +126,8 @@ export class GitService { // explicitly specified or defaulted to the repository's HEAD const branchArgs = this.manager.buildGetCurrentBranchArgs(); const branchCommand = this.buildCommand(branchArgs); - const branchExecResult = await this.sessionManager.executeInSession(sessionId, branchCommand, { cwd: targetDirectory }); + // Quick operation - 10 second timeout + const branchExecResult = await this.sessionManager.executeInSession(sessionId, branchCommand, { cwd: targetDirectory, timeoutMs: 10000 }); if (!branchExecResult.success) { // If we can't get the branch, use fallback but don't fail the entire operation @@ -221,7 +223,8 @@ export class GitService { const command = this.buildCommand(args); // Execute git checkout (via SessionManager) - const execResult = await this.sessionManager.executeInSession(sessionId, command, { cwd: repoPath }); + // Checkout can take time with large repos - 30 second timeout + const execResult = await this.sessionManager.executeInSession(sessionId, command, { cwd: repoPath, timeoutMs: 30000 }); if (!execResult.success) { return execResult as ServiceResult; @@ -301,7 +304,8 @@ export class GitService { const command = this.buildCommand(args); // Execute command (via SessionManager) - const execResult = await this.sessionManager.executeInSession(sessionId, command, { cwd: repoPath }); + // Quick operation - 10 second timeout + const execResult = await this.sessionManager.executeInSession(sessionId, command, { cwd: repoPath, timeoutMs: 10000 }); if (!execResult.success) { return execResult as ServiceResult; @@ -375,7 +379,8 @@ export class GitService { const command = this.buildCommand(args); // Execute command (via SessionManager) - const execResult = await this.sessionManager.executeInSession(sessionId, command, { cwd: repoPath }); + // Quick operation - 10 second timeout + const execResult = await this.sessionManager.executeInSession(sessionId, command, { cwd: repoPath, timeoutMs: 10000 }); if (!execResult.success) { return execResult as ServiceResult; diff --git a/packages/sandbox-container/src/services/session-manager.ts b/packages/sandbox-container/src/services/session-manager.ts index 8f2e4ded..a3c46f9a 100644 --- a/packages/sandbox-container/src/services/session-manager.ts +++ b/packages/sandbox-container/src/services/session-manager.ts @@ -130,7 +130,12 @@ export class SessionManager { const session = sessionResult.data; - const result = await session.exec(command, options?.cwd ? { cwd: options.cwd } : undefined); + // Pass both cwd and timeout to session.exec() + const execOptions: { cwd?: string; timeoutMs?: number } = {}; + if (options?.cwd) execOptions.cwd = options.cwd; + if (options?.timeoutMs !== undefined) execOptions.timeoutMs = options.timeoutMs; + + const result = await session.exec(command, Object.keys(execOptions).length > 0 ? execOptions : undefined); return { success: true, diff --git a/packages/sandbox-container/src/session.ts b/packages/sandbox-container/src/session.ts index 788d4db1..ff2b1a49 100644 --- a/packages/sandbox-container/src/session.ts +++ b/packages/sandbox-container/src/session.ts @@ -88,6 +88,8 @@ export interface RawExecResult { interface ExecOptions { /** Override working directory for this command only */ cwd?: string; + /** Override timeout for this command only (milliseconds) */ + timeoutMs?: number; } /** Command handle for tracking and killing running commands */ @@ -230,7 +232,7 @@ export class Session { // 2. Shell death (shell process exits unexpectedly) // This allows us to detect shell termination (e.g., from 'exit' command) immediately const exitCode = await Promise.race([ - this.waitForExitCode(exitCodeFile), + this.waitForExitCode(exitCodeFile, options?.timeoutMs), this.shellExitedPromise! ]); @@ -737,8 +739,10 @@ export class Session { * * Uses fs.watch for fast detection, with polling fallback for systems where * fs.watch doesn't reliably detect rename() operations (common on tmpfs, overlayfs). + * + * @param timeoutMs Optional per-command timeout override (milliseconds) */ - private async waitForExitCode(exitCodeFile: string): Promise { + private async waitForExitCode(exitCodeFile: string, timeoutMs?: number): Promise { return new Promise((resolve, reject) => { const dir = dirname(exitCodeFile); const filename = basename(exitCodeFile); @@ -781,15 +785,17 @@ export class Session { }, 50); // Poll every 50ms as fallback // STEP 3: Set up timeout if configured - if (this.commandTimeoutMs !== undefined) { + // Use per-command timeout if provided, otherwise use session-level default + const effectiveTimeout = timeoutMs ?? this.commandTimeoutMs; + if (effectiveTimeout !== undefined) { setTimeout(() => { if (!resolved) { resolved = true; watcher.close(); clearInterval(pollInterval); - reject(new Error(`Command timeout after ${this.commandTimeoutMs}ms`)); + reject(new Error(`Command timeout after ${effectiveTimeout}ms`)); } - }, this.commandTimeoutMs); + }, effectiveTimeout); } // STEP 4: Check if file already exists diff --git a/packages/sandbox-container/tests/services/git-service.test.ts b/packages/sandbox-container/tests/services/git-service.test.ts index 156f9ab5..7a333bcd 100644 --- a/packages/sandbox-container/tests/services/git-service.test.ts +++ b/packages/sandbox-container/tests/services/git-service.test.ts @@ -85,19 +85,20 @@ describe('GitService', () => { expect(mockSecurityService.validateGitUrl).toHaveBeenCalledWith('https://github.com/user/repo.git'); expect(mockSecurityService.validatePath).toHaveBeenCalledWith('/workspace/repo'); - // Verify SessionManager was called for git clone (cwd is undefined) + // Verify SessionManager was called for git clone with 5 minute timeout expect(mockSessionManager.executeInSession).toHaveBeenNthCalledWith( 1, 'default', - 'git clone https://github.com/user/repo.git /workspace/repo' + 'git clone https://github.com/user/repo.git /workspace/repo', + { timeoutMs: 300000 } ); - // Verify SessionManager was called for getting current branch + // Verify SessionManager was called for getting current branch with 10 second timeout expect(mockSessionManager.executeInSession).toHaveBeenNthCalledWith( 2, 'default', 'git branch --show-current', - { cwd: '/workspace/repo' } + { cwd: '/workspace/repo', timeoutMs: 10000 } ); }); @@ -134,11 +135,12 @@ describe('GitService', () => { expect(result.data.branch).toBe('develop'); } - // Verify git clone command includes branch option (cwd is undefined) + // Verify git clone command includes branch option with 5 minute timeout expect(mockSessionManager.executeInSession).toHaveBeenNthCalledWith( 1, 'session-123', - 'git clone --branch develop https://github.com/user/repo.git /tmp/custom-target' + 'git clone --branch develop https://github.com/user/repo.git /tmp/custom-target', + { timeoutMs: 300000 } ); }); @@ -237,11 +239,11 @@ describe('GitService', () => { expect(result.success).toBe(true); - // Verify SessionManager was called with correct parameters + // Verify SessionManager was called with correct parameters (30s timeout) expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( 'session-123', 'git checkout develop', - { cwd: '/tmp/repo' } + { cwd: '/tmp/repo', timeoutMs: 30000 } ); }); @@ -298,7 +300,7 @@ describe('GitService', () => { expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( 'session-123', 'git branch --show-current', - { cwd: '/tmp/repo' } + { cwd: '/tmp/repo', timeoutMs: 10000 } ); }); }); @@ -341,7 +343,7 @@ describe('GitService', () => { expect(mockSessionManager.executeInSession).toHaveBeenCalledWith( 'session-123', 'git branch -a', - { cwd: '/tmp/repo' } + { cwd: '/tmp/repo', timeoutMs: 10000 } ); });