Skip to content

Commit 73e55ca

Browse files
committed
feat: preserve tmpfile overflow logs with persistent temp directories
- Add persistentTempDir option to RunScriptOptions interface - Implement conditional cleanup that skips deletion when tmpfile overflow occurs - Pass persistentTempDir from tools.ts to enable overflow log preservation - Add comprehensive test coverage for overflow and cleanup scenarios - Clean up temp directories in error paths to prevent disk leaks Addresses Codex review comments: - P1: Preserve tmpfile overflow logs for script tools (scriptRunner.ts:224) - P2: Clean up temp dirs when script execution fails (scriptRunner.ts:248) Change-Id: I92e1492c55294bb4a5cfaaf1e572724b Signed-off-by: Thomas Kosiewski <[email protected]>
1 parent c70e086 commit 73e55ca

File tree

5 files changed

+201
-28
lines changed

5 files changed

+201
-28
lines changed

src/browser/components/AIView.tsx

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -533,26 +533,10 @@ const AIViewInner: React.FC<AIViewProps> = ({
533533
<PinnedTodoList workspaceId={workspaceId} />
534534
{canInterrupt && (
535535
<StreamingBarrier
536-
statusText={
537-
isCompacting
538-
? currentModel
539-
? `${getModelName(currentModel)} compacting...`
540-
: "compacting..."
541-
: currentModel
542-
? `${getModelName(currentModel)} streaming...`
543-
: "streaming..."
544-
}
545-
cancelText={`hit ${formatKeybind(vimEnabled ? KEYBINDS.INTERRUPT_STREAM_VIM : KEYBINDS.INTERRUPT_STREAM_NORMAL)} to cancel`}
546-
tokenCount={
547-
activeStreamMessageId
548-
? aggregator.getStreamingTokenCount(activeStreamMessageId)
549-
: undefined
550-
}
551-
tps={
552-
activeStreamMessageId
553-
? aggregator.getStreamingTPS(activeStreamMessageId)
554-
: undefined
555-
}
536+
statusText={streamingStatusText}
537+
cancelText={streamingCancelText}
538+
tokenCount={streamingTokenCount}
539+
tps={streamingTPS}
556540
/>
557541
)}
558542
{workspaceState?.queuedMessage && (

src/common/utils/tools/tools.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ describe("getToolsForModel", () => {
167167
[],
168168
expect.objectContaining({
169169
overflowPolicy: "tmpfile",
170+
persistentTempDir: config.runtimeTempDir,
170171
})
171172
);
172173

src/common/utils/tools/tools.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ export async function getToolsForModel(
137137
secrets: config.secrets ?? {},
138138
timeoutSecs: 300,
139139
overflowPolicy: "tmpfile",
140+
persistentTempDir: config.runtimeTempDir,
140141
}
141142
);
142143

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
import { describe, test, expect } from "bun:test";
2+
import * as path from "path";
3+
import * as os from "os";
4+
import { promises as fsPromises } from "fs";
5+
6+
import { runWorkspaceScript } from "@/node/services/scriptRunner";
7+
import { LocalRuntime } from "@/node/runtime/LocalRuntime";
8+
9+
interface WorkspaceContext {
10+
workspacePath: string;
11+
persistentRoot: string;
12+
runtime: LocalRuntime;
13+
cleanup: () => Promise<void>;
14+
}
15+
16+
async function createWorkspaceWithScript(
17+
scriptName: string,
18+
scriptContents: string
19+
): Promise<WorkspaceContext> {
20+
const workspacePath = await fsPromises.mkdtemp(path.join(os.tmpdir(), "mux-script-runner-"));
21+
22+
const scriptsDir = path.join(workspacePath, ".mux", "scripts");
23+
await fsPromises.mkdir(scriptsDir, { recursive: true });
24+
25+
const scriptPath = path.join(scriptsDir, scriptName);
26+
await fsPromises.writeFile(scriptPath, scriptContents, { mode: 0o755 });
27+
await fsPromises.chmod(scriptPath, 0o755);
28+
29+
const persistentRoot = path.join(workspacePath, ".mux-temp-root");
30+
await fsPromises.mkdir(persistentRoot, { recursive: true });
31+
32+
const runtime = new LocalRuntime(path.dirname(workspacePath));
33+
34+
const cleanup = async () => {
35+
await fsPromises.rm(workspacePath, { recursive: true, force: true });
36+
};
37+
38+
return { workspacePath, persistentRoot, runtime, cleanup };
39+
}
40+
41+
function extractOverflowPath(errorText: string): string | undefined {
42+
const match = /saved to (.+)/.exec(errorText);
43+
return match?.[1]?.trim();
44+
}
45+
46+
async function waitForDirEmpty(dir: string, timeoutMs = 2000): Promise<boolean> {
47+
const deadline = Date.now() + timeoutMs;
48+
while (Date.now() < deadline) {
49+
try {
50+
const entries = await fsPromises.readdir(dir);
51+
if (entries.length === 0) {
52+
return true;
53+
}
54+
} catch (error) {
55+
if ((error as NodeJS.ErrnoException).code === "ENOENT") {
56+
return true;
57+
}
58+
throw error;
59+
}
60+
await new Promise((resolve) => setTimeout(resolve, 50));
61+
}
62+
63+
try {
64+
const entries = await fsPromises.readdir(dir);
65+
return entries.length === 0;
66+
} catch (error) {
67+
if ((error as NodeJS.ErrnoException).code === "ENOENT") {
68+
return true;
69+
}
70+
throw error;
71+
}
72+
}
73+
74+
describe("runWorkspaceScript persistent temp directory handling", () => {
75+
const overflowScript = `#!/usr/bin/env bash
76+
set -euo pipefail
77+
node - <<'NODE'
78+
const chunk = '0123456789'.repeat(200);
79+
for (let i = 0; i < 400; i++) {
80+
console.log(chunk);
81+
}
82+
NODE
83+
`;
84+
85+
const simpleScript = `#!/usr/bin/env bash
86+
set -euo pipefail
87+
echo "done"
88+
`;
89+
90+
test("preserves tmpfile overflow logs when persistent dir is provided", async () => {
91+
const context = await createWorkspaceWithScript("overflow", overflowScript);
92+
const { workspacePath, persistentRoot, runtime, cleanup } = context;
93+
94+
try {
95+
const result = await runWorkspaceScript(runtime, workspacePath, "overflow", [], {
96+
overflowPolicy: "tmpfile",
97+
persistentTempDir: persistentRoot,
98+
});
99+
100+
expect(result.success).toBe(true);
101+
if (!result.success) {
102+
throw new Error(`Expected success, got error: ${result.error}`);
103+
}
104+
const toolResult = result.data.toolResult;
105+
expect(toolResult.success).toBe(false);
106+
if (toolResult.success) {
107+
throw new Error("Expected bash tool failure for overflow scenario");
108+
}
109+
expect(toolResult.error).toContain("OUTPUT OVERFLOW");
110+
111+
const overflowPath = extractOverflowPath(toolResult.error ?? "");
112+
expect(overflowPath).toBeTruthy();
113+
await fsPromises.access(overflowPath!);
114+
} finally {
115+
await fsPromises.rm(persistentRoot, { recursive: true, force: true });
116+
await cleanup();
117+
}
118+
});
119+
120+
test("cleans persistent temp subdirectories when no overflow occurs", async () => {
121+
const context = await createWorkspaceWithScript("light", simpleScript);
122+
const { workspacePath, persistentRoot, runtime, cleanup } = context;
123+
124+
try {
125+
const result = await runWorkspaceScript(runtime, workspacePath, "light", [], {
126+
overflowPolicy: "tmpfile",
127+
persistentTempDir: persistentRoot,
128+
});
129+
130+
expect(result.success).toBe(true);
131+
if (!result.success) {
132+
throw new Error(`Expected success, got error: ${result.error}`);
133+
}
134+
expect(result.data.toolResult.success).toBe(true);
135+
136+
const emptied = await waitForDirEmpty(persistentRoot);
137+
expect(emptied).toBe(true);
138+
} finally {
139+
await cleanup();
140+
}
141+
});
142+
});

src/node/services/scriptRunner.ts

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ export interface RunScriptOptions {
3636
timeoutSecs?: number;
3737
abortSignal?: AbortSignal;
3838
overflowPolicy?: "truncate" | "tmpfile";
39+
/**
40+
* Optional persistent temp directory root (e.g., stream-scoped ~/.mux-tmp/<token>).
41+
* When provided, scriptRunner will place its temp files in a unique subdirectory inside
42+
* this root so overflow logs can survive until the stream-level cleanup runs.
43+
*/
44+
persistentTempDir?: string;
3945
}
4046

4147
/**
@@ -55,6 +61,7 @@ export async function runWorkspaceScript(
5561
timeoutSecs = 300,
5662
abortSignal,
5763
overflowPolicy = "truncate",
64+
persistentTempDir,
5865
} = options;
5966

6067
// 1. Validate script name safely
@@ -120,12 +127,24 @@ export async function runWorkspaceScript(
120127
}
121128

122129
// 3. Prepare temporary environment (MUX_OUTPUT, MUX_PROMPT)
123-
// Create a temp directory for this execution context
124-
const tempDirResult = await execBuffered(
125-
runtime,
126-
"mktemp -d 2>/dev/null || mktemp -d -t 'mux-script'",
127-
{ cwd: workspacePath, timeout: 5 }
128-
);
130+
// Create a temp directory for this execution context. When a persistent temp root is provided,
131+
// create a unique subdirectory inside it so overflow logs survive until stream cleanup.
132+
const normalizeForShell = (value: string): string => value.replace(/\\/g, "/");
133+
const escapeSingleQuotes = (value: string): string => value.replace(/'/g, "'\\''");
134+
135+
const persistentBase =
136+
persistentTempDir && persistentTempDir.trim().length > 0
137+
? normalizeForShell(persistentTempDir.trim()).replace(/\/+$/, "")
138+
: undefined;
139+
140+
const tempDirCommand = persistentBase
141+
? `mkdir -p '${escapeSingleQuotes(persistentBase)}' && mktemp -d '${escapeSingleQuotes(`${persistentBase}/script-XXXXXX`)}'`
142+
: "mktemp -d 2>/dev/null || mktemp -d -t 'mux-script'";
143+
144+
const tempDirResult = await execBuffered(runtime, tempDirCommand, {
145+
cwd: workspacePath,
146+
timeout: 5,
147+
});
129148

130149
if (tempDirResult.exitCode !== 0) {
131150
return Err(`Failed to prepare script environment: ${tempDirResult.stderr || "mkdir failed"}`);
@@ -136,13 +155,28 @@ export async function runWorkspaceScript(
136155
return Err("Failed to prepare script environment: runtime temp directory was empty");
137156
}
138157

158+
let skipCleanup = false;
159+
let cleanupScheduled = false;
160+
const cleanupTempDir = (): void => {
161+
if (skipCleanup || cleanupScheduled) {
162+
return;
163+
}
164+
cleanupScheduled = true;
165+
const safeTempDir = runtimeTempDir.replace(/"/g, '\\"');
166+
void execBuffered(runtime, `rm -rf "${safeTempDir}"`, {
167+
cwd: workspacePath,
168+
timeout: 5,
169+
});
170+
};
171+
139172
const outputFile = path.posix.join(runtimeTempDir, "output.txt");
140173
const promptFile = path.posix.join(runtimeTempDir, "prompt.txt");
141174

142175
try {
143176
await writeFileString(runtime, outputFile, "");
144177
await writeFileString(runtime, promptFile, "");
145178
} catch (prepError) {
179+
cleanupTempDir();
146180
return Err(
147181
`Failed to prepare script environment files: ${
148182
prepError instanceof Error ? prepError.message : String(prepError)
@@ -220,8 +254,18 @@ export async function runWorkspaceScript(
220254
/* ignore */
221255
}
222256

223-
// 7. Cleanup (best effort)
224-
void execBuffered(runtime, `rm -rf "${runtimeTempDir}"`, { cwd: workspacePath, timeout: 5 });
257+
const indicatesTmpfileOverflow =
258+
Boolean(persistentBase) &&
259+
overflowPolicy === "tmpfile" &&
260+
!toolResult.success &&
261+
typeof toolResult.error === "string" &&
262+
toolResult.error.includes("[OUTPUT OVERFLOW -");
263+
264+
if (indicatesTmpfileOverflow) {
265+
skipCleanup = true;
266+
} else {
267+
cleanupTempDir();
268+
}
225269

226270
// Extract stdout/stderr based on success/failure
227271
let stdout = "";
@@ -243,6 +287,7 @@ export async function runWorkspaceScript(
243287
toolResult,
244288
});
245289
} catch (execError) {
290+
cleanupTempDir();
246291
return Err(
247292
`Script execution failed: ${execError instanceof Error ? execError.message : String(execError)}`
248293
);

0 commit comments

Comments
 (0)