Skip to content

Commit 8d8d8f1

Browse files
committed
Fix bash overflow temp directory to use Runtime abstraction
The bash tool overflow feature writes large outputs to temp files when they exceed display limits. The previous implementation broke with SSH runtimes because temp directories were created using local filesystem operations (fs.mkdirSync) but passed to runtime.writeFile() which needs runtime-appropriate paths. Changes: - Renamed 'tempDir' to 'runtimeTempDir' throughout for clarity - StreamManager.createTempDirForStream() now uses runtime.exec('mkdir -p') - Stream cleanup now uses runtime.exec('rm -rf') for temp directory removal - Pass Runtime parameter through startStream() to enable temp dir operations - Store Runtime in WorkspaceStreamInfo for cleanup access - Updated ToolConfiguration interface (tempDir -> runtimeTempDir) - Updated all tests to use runtimeTempDir This ensures temp directories are created and cleaned up in the correct location (local for LocalRuntime, remote for SSHRuntime) while preserving the existing cleanup guarantees when streams end.
1 parent 88b0c80 commit 8d8d8f1

File tree

5 files changed

+64
-36
lines changed

5 files changed

+64
-36
lines changed

src/services/aiService.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -522,14 +522,17 @@ export class AIService extends EventEmitter {
522522

523523
// Generate stream token and create temp directory for tools
524524
const streamToken = this.streamManager.generateStreamToken();
525-
const tempDir = this.streamManager.createTempDirForStream(streamToken);
525+
const runtimeTempDir = await this.streamManager.createTempDirForStream(
526+
streamToken,
527+
runtime
528+
);
526529

527530
// Get model-specific tools with workspace path (correct for local or remote)
528531
const allTools = await getToolsForModel(modelString, {
529532
cwd: workspacePath,
530533
runtime,
531534
secrets: secretsToRecord(projectSecrets),
532-
tempDir,
535+
runtimeTempDir,
533536
});
534537

535538
// Apply tool policy to filter tools (if policy provided)
@@ -694,6 +697,7 @@ export class AIService extends EventEmitter {
694697
modelString,
695698
historySequence,
696699
systemMessage,
700+
runtime,
697701
abortSignal,
698702
tools,
699703
{

src/services/streamManager.ts

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import type { HistoryService } from "./historyService";
3131
import { AsyncMutex } from "@/utils/concurrency/asyncMutex";
3232
import type { ToolPolicy } from "@/utils/tools/toolPolicy";
3333
import { StreamingTokenTracker } from "@/utils/main/StreamingTokenTracker";
34+
import type { Runtime } from "@/runtime/Runtime";
3435

3536
// Type definitions for stream parts with extended properties
3637
interface ReasoningDeltaPart {
@@ -107,7 +108,9 @@ interface WorkspaceStreamInfo {
107108
// Track background processing promise for guaranteed cleanup
108109
processingPromise: Promise<void>;
109110
// Temporary directory for tool outputs (auto-cleaned when stream ends)
110-
tempDir: string;
111+
runtimeTempDir: string;
112+
// Runtime for temp directory cleanup
113+
runtime: Runtime;
111114
}
112115

113116
/**
@@ -242,11 +245,24 @@ export class StreamManager extends EventEmitter {
242245
* - Agent mistakes when copying/manipulating paths
243246
* - Harder to read in tool outputs
244247
* - Potential path length issues on some systems
248+
*
249+
* Uses the Runtime abstraction so temp directories work for both local and SSH runtimes.
245250
*/
246-
public createTempDirForStream(streamToken: StreamToken): string {
247-
const homeDir = os.homedir();
248-
const tempDir = path.join(homeDir, ".cmux-tmp", streamToken);
249-
fs.mkdirSync(tempDir, { recursive: true, mode: 0o700 });
251+
public async createTempDirForStream(
252+
streamToken: StreamToken,
253+
runtime: Runtime
254+
): Promise<string> {
255+
const tempDir = path.join("~", ".cmux-tmp", streamToken);
256+
// Use runtime.exec() to create directory (works for both local and remote)
257+
const result = await runtime.exec(`mkdir -p "${tempDir}"`, {
258+
cwd: "~",
259+
timeout: 10,
260+
});
261+
// Wait for command to complete
262+
const exitCode = await result.exitCode;
263+
if (exitCode !== 0) {
264+
throw new Error(`Failed to create temp directory ${tempDir}: exit code ${exitCode}`);
265+
}
250266
return tempDir;
251267
}
252268

@@ -429,7 +445,8 @@ export class StreamManager extends EventEmitter {
429445
private createStreamAtomically(
430446
workspaceId: WorkspaceId,
431447
streamToken: StreamToken,
432-
tempDir: string,
448+
runtimeTempDir: string,
449+
runtime: Runtime,
433450
messages: ModelMessage[],
434451
model: LanguageModel,
435452
modelString: string,
@@ -508,7 +525,8 @@ export class StreamManager extends EventEmitter {
508525
lastPartialWriteTime: 0, // Initialize to 0 to allow immediate first write
509526
partialWritePromise: undefined, // No write in flight initially
510527
processingPromise: Promise.resolve(), // Placeholder, overwritten in startStream
511-
tempDir, // Stream-scoped temp directory for tool outputs
528+
runtimeTempDir, // Stream-scoped temp directory for tool outputs
529+
runtime, // Runtime for temp directory cleanup
512530
};
513531

514532
// Atomically register the stream
@@ -961,13 +979,20 @@ export class StreamManager extends EventEmitter {
961979
streamInfo.partialWriteTimer = undefined;
962980
}
963981

964-
// Clean up stream temp directory
965-
if (streamInfo.tempDir && fs.existsSync(streamInfo.tempDir)) {
982+
// Clean up stream temp directory using runtime
983+
if (streamInfo.runtimeTempDir) {
966984
try {
967-
fs.rmSync(streamInfo.tempDir, { recursive: true, force: true });
968-
log.debug(`Cleaned up temp dir: ${streamInfo.tempDir}`);
985+
const result = await streamInfo.runtime.exec(
986+
`rm -rf "${streamInfo.runtimeTempDir}"`,
987+
{
988+
cwd: "~",
989+
timeout: 10,
990+
}
991+
);
992+
await result.exitCode; // Wait for completion
993+
log.debug(`Cleaned up temp dir: ${streamInfo.runtimeTempDir}`);
969994
} catch (error) {
970-
log.error(`Failed to cleanup temp dir ${streamInfo.tempDir}:`, error);
995+
log.error(`Failed to cleanup temp dir ${streamInfo.runtimeTempDir}:`, error);
971996
// Don't throw - cleanup is best-effort
972997
}
973998
}
@@ -1090,6 +1115,7 @@ export class StreamManager extends EventEmitter {
10901115
modelString: string,
10911116
historySequence: number,
10921117
system: string,
1118+
runtime: Runtime,
10931119
abortSignal?: AbortSignal,
10941120
tools?: Record<string, Tool>,
10951121
initialMetadata?: Partial<CmuxMetadata>,
@@ -1123,18 +1149,16 @@ export class StreamManager extends EventEmitter {
11231149
// Step 2: Use provided stream token or generate a new one
11241150
const streamToken = providedStreamToken ?? this.generateStreamToken();
11251151

1126-
// Step 3: Create temp directory for this stream
1127-
// If token was provided, temp dir might already exist - that's fine
1128-
const tempDir = path.join(os.homedir(), ".cmux-tmp", streamToken);
1129-
if (!fs.existsSync(tempDir)) {
1130-
fs.mkdirSync(tempDir, { recursive: true, mode: 0o700 });
1131-
}
1152+
// Step 3: Create temp directory for this stream using runtime
1153+
// If token was provided, temp dir might already exist - mkdir -p handles this
1154+
const runtimeTempDir = await this.createTempDirForStream(streamToken, runtime);
11321155

11331156
// Step 4: Atomic stream creation and registration
11341157
const streamInfo = this.createStreamAtomically(
11351158
typedWorkspaceId,
11361159
streamToken,
1137-
tempDir,
1160+
runtimeTempDir,
1161+
runtime,
11381162
messages,
11391163
model,
11401164
modelString,

src/services/tools/bash.test.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ function createTestBashTool(options?: { niceness?: number }) {
2323
const tool = createBashTool({
2424
cwd: process.cwd(),
2525
runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }),
26-
tempDir: tempDir.path,
26+
runtimeTempDir: tempDir.path,
2727
...options,
2828
});
2929

@@ -165,7 +165,7 @@ describe("bash tool", () => {
165165
const tool = createBashTool({
166166
cwd: process.cwd(),
167167
runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }),
168-
tempDir: tempDir.path,
168+
runtimeTempDir: tempDir.path,
169169
overflow_policy: "truncate",
170170
});
171171

@@ -204,7 +204,7 @@ describe("bash tool", () => {
204204
const tool = createBashTool({
205205
cwd: process.cwd(),
206206
runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }),
207-
tempDir: tempDir.path,
207+
runtimeTempDir: tempDir.path,
208208
overflow_policy: "truncate",
209209
});
210210

@@ -236,7 +236,7 @@ describe("bash tool", () => {
236236
const tool = createBashTool({
237237
cwd: process.cwd(),
238238
runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }),
239-
tempDir: tempDir.path,
239+
runtimeTempDir: tempDir.path,
240240
overflow_policy: "truncate",
241241
});
242242

@@ -272,7 +272,7 @@ describe("bash tool", () => {
272272
const tool = createBashTool({
273273
cwd: tempDir.path, // Use tempDir as cwd so we can verify file creation
274274
runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }),
275-
tempDir: tempDir.path,
275+
runtimeTempDir: tempDir.path,
276276
// overflow_policy not specified - should default to tmpfile
277277
});
278278

@@ -306,7 +306,7 @@ describe("bash tool", () => {
306306
const tool = createBashTool({
307307
cwd: process.cwd(),
308308
runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }),
309-
tempDir: tempDir.path,
309+
runtimeTempDir: tempDir.path,
310310
});
311311

312312
// Generate ~50KB of output (well over 16KB display limit, under 100KB file limit)
@@ -358,7 +358,7 @@ describe("bash tool", () => {
358358
const tool = createBashTool({
359359
cwd: process.cwd(),
360360
runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }),
361-
tempDir: tempDir.path,
361+
runtimeTempDir: tempDir.path,
362362
});
363363

364364
// Generate ~150KB of output (exceeds 100KB file limit)
@@ -401,7 +401,7 @@ describe("bash tool", () => {
401401
const tool = createBashTool({
402402
cwd: process.cwd(),
403403
runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }),
404-
tempDir: tempDir.path,
404+
runtimeTempDir: tempDir.path,
405405
});
406406

407407
// Generate output that exceeds display limit but not file limit
@@ -443,7 +443,7 @@ describe("bash tool", () => {
443443
const tool = createBashTool({
444444
cwd: process.cwd(),
445445
runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }),
446-
tempDir: tempDir.path,
446+
runtimeTempDir: tempDir.path,
447447
});
448448

449449
// Generate a single line exceeding 1KB limit, then try to output more
@@ -483,7 +483,7 @@ describe("bash tool", () => {
483483
const tool = createBashTool({
484484
cwd: process.cwd(),
485485
runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }),
486-
tempDir: tempDir.path,
486+
runtimeTempDir: tempDir.path,
487487
});
488488

489489
// Generate ~15KB of output (just under 16KB display limit)
@@ -513,7 +513,7 @@ describe("bash tool", () => {
513513
const tool = createBashTool({
514514
cwd: process.cwd(),
515515
runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }),
516-
tempDir: tempDir.path,
516+
runtimeTempDir: tempDir.path,
517517
});
518518

519519
// Generate exactly 300 lines (hits line limit exactly)

src/services/tools/bash.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -421,9 +421,9 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => {
421421
try {
422422
// Use 8 hex characters for short, memorable temp file IDs
423423
const fileId = Math.random().toString(16).substring(2, 10);
424-
// Write to .cmux/tmp directory relative to cwd for runtime-agnostic path
424+
// Write to runtime temp directory (managed by StreamManager)
425425
// This works for both LocalRuntime (local path) and SSHRuntime (remote path)
426-
const overflowPath = path.join(config.cwd, ".cmux", "tmp", `bash-${fileId}.txt`);
426+
const overflowPath = path.join(config.runtimeTempDir, `bash-${fileId}.txt`);
427427
const fullOutput = lines.join("\n");
428428

429429
// Use runtime.writeFile() for SSH support

src/utils/tools/tools.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ export interface ToolConfiguration {
2222
secrets?: Record<string, string>;
2323
/** Process niceness level (optional, -20 to 19, lower = higher priority) */
2424
niceness?: number;
25-
/** Temporary directory for tool outputs (required) */
26-
tempDir: string;
25+
/** Temporary directory for tool outputs in runtime's context (local or remote) */
26+
runtimeTempDir: string;
2727
/** Overflow policy for bash tool output (optional, not exposed to AI) */
2828
overflow_policy?: "truncate" | "tmpfile";
2929
}

0 commit comments

Comments
 (0)