feat: enhance Bash tool for background task cleanup logic

Author: qorzj

src/session.ts

@@ -8,6 +8,7 @@ import type { ChatCompletionMessageParam, ChatCompletionContentPart } from "open
 import { launchNotifyScript } from "./common/notify";
 import { buildThinkingRequestOptions } from "./common/openai-thinking";
 import { DEEPSEEK_V4_MODELS, supportsMultimodal } from "./common/model-capabilities";
+import { readTextFileWithMetadata } from "./common/file-utils";
 import {
   getCompactPrompt,
   getDefaultSkillPrompt,
@@ -60,6 +61,7 @@ export type {
 const MAX_SESSION_ENTRIES = 50;
 const MAX_PROJECT_CODE_LENGTH = 64;
 const PROJECT_CODE_HASH_LENGTH = 16;
+const BACKGROUND_FAILURE_LOG_TAIL_CHARS = 4000;
 const DEFAULT_COMPACT_PROMPT_TOKEN_THRESHOLD = 128 * 1024;
 const DEEPSEEK_V4_COMPACT_PROMPT_TOKEN_THRESHOLD = 512 * 1024;
 
@@ -330,6 +332,7 @@ export class SessionManager {
   private activePromptController: AbortController | null = null;
   private readonly sessionControllers = new Map<string, AbortController>();
   private readonly processTimeoutControls = new Map<string, ProcessTimeoutControl>();
+  private readonly liveProcessKeys = new Set<string>();
   private readonly toolExecutor: ToolExecutor;
   private readonly mcpManager = new McpManager();
   private mcpToolDefinitions: ToolDefinition[] = [];
@@ -379,6 +382,7 @@ export class SessionManager {
         sessionController.abort();
       }
     }
+    this.killLiveProcesses();
     this.sessionControllers.clear();
     this.processTimeoutControls.clear();
     this.mcpManager.disconnect();
@@ -1525,7 +1529,9 @@ ${skillMd}
     const killedPids: number[] = [];
     const failedPids: number[] = [];
     for (const pid of processIds) {
-      this.processTimeoutControls.delete(this.getProcessControlKey(sessionId, pid));
+      const processControlKey = this.getProcessControlKey(sessionId, pid);
+      this.processTimeoutControls.delete(processControlKey);
+      this.liveProcessKeys.delete(processControlKey);
       if (killProcessTree(pid, "SIGKILL")) {
         killedPids.push(pid);
         continue;
@@ -1892,21 +1898,11 @@ ${skillMd}
     const processIds = options.processIds ?? [];
     for (const pid of processIds) {
       const processControlKey = this.getProcessControlKey(sessionId, pid);
-      if (!this.processTimeoutControls.has(processControlKey)) {
+      if (!this.processTimeoutControls.has(processControlKey) && !this.liveProcessKeys.has(processControlKey)) {
         continue;
       }
 
-      const killedGroup = killProcessTree(pid, "SIGKILL");
-      if (killedGroup) {
-        this.processTimeoutControls.delete(processControlKey);
-        continue;
-      }
-      try {
-        process.kill(pid, "SIGKILL");
-      } catch {
-        // ignore process-kill failures during cleanup
-      }
-      this.processTimeoutControls.delete(processControlKey);
+      this.killTrackedProcess(processControlKey, pid);
     }
 
     clearSessionState(sessionId);
@@ -2667,6 +2663,7 @@ ${skillMd}
 
   private addSessionProcess(sessionId: string, processId: string | number, command: string): void {
     const now = new Date().toISOString();
+    this.liveProcessKeys.add(this.getProcessControlKey(sessionId, processId));
     this.updateSessionEntry(sessionId, (entry) => {
       const processes = new Map(entry.processes ?? []);
       processes.set(String(processId), { startTime: now, command });
@@ -2699,12 +2696,47 @@ ${skillMd}
           ? `signal ${completion.signal}`
           : completion.error || "unknown status";
     const durationMs = Math.max(0, completion.completedAtMs - completion.startedAtMs);
-    const content =
+    const baseContent =
       `Background command "${completion.command}" ${status} with ${exitText} ` +
       `after ${this.formatBackgroundDuration(durationMs)}. Output: ${completion.outputPath}`;
+    const logTail = completion.ok ? null : this.buildBackgroundFailureLogTailSlice(completion.outputPath);
+    const content = logTail ? `${baseContent}\n${logTail}` : baseContent;
     this.addSessionSystemMessage(sessionId, content, true);
   }
 
+  private buildBackgroundFailureLogTailSlice(outputPath: string): string | null {
+    const tail = this.readTextFileTail(outputPath, BACKGROUND_FAILURE_LOG_TAIL_CHARS);
+    if (!tail || !tail.content) {
+      return null;
+    }
+    const prefix = tail.truncated ? `(${tail.totalBytes} bytes)...\n` : "";
+    return [
+      `<background_task_failure_log path="${outputPath}">`,
+      `${prefix}${tail.content}`,
+      "</background_task_failure_log>",
+    ].join("\n");
+  }
+
+  private readTextFileTail(
+    filePath: string,
+    maxChars: number
+  ): { content: string; totalBytes: number; truncated: boolean } | null {
+    try {
+      const stat = fs.statSync(filePath);
+      if (!stat.isFile() || stat.size <= 0) {
+        return null;
+      }
+      const content = readTextFileWithMetadata(filePath).content;
+      return {
+        content: content.slice(-maxChars).trimEnd(),
+        totalBytes: stat.size,
+        truncated: content.length > maxChars,
+      };
+    } catch {
+      return null;
+    }
+  }
+
   private formatBackgroundDuration(durationMs: number): string {
     if (durationMs < 1000) {
       return `${durationMs}ms`;
@@ -2720,7 +2752,9 @@ ${skillMd}
 
   private removeSessionProcess(sessionId: string, processId: string | number): void {
     const now = new Date().toISOString();
-    this.processTimeoutControls.delete(this.getProcessControlKey(sessionId, processId));
+    const processControlKey = this.getProcessControlKey(sessionId, processId);
+    this.processTimeoutControls.delete(processControlKey);
+    this.liveProcessKeys.delete(processControlKey);
     this.updateSessionEntry(sessionId, (entry) => {
       const processes = new Map(entry.processes ?? []);
       processes.delete(String(processId));
@@ -2783,6 +2817,37 @@ ${skillMd}
     return `${sessionId}:${String(processId)}`;
   }
 
+  private killLiveProcesses(): void {
+    for (const processControlKey of Array.from(this.liveProcessKeys)) {
+      const processId = this.getProcessIdFromControlKey(processControlKey);
+      if (processId === null) {
+        this.liveProcessKeys.delete(processControlKey);
+        continue;
+      }
+      this.killTrackedProcess(processControlKey, processId);
+    }
+  }
+
+  private killTrackedProcess(processControlKey: string, processId: number): void {
+    const killedGroup = killProcessTree(processId, "SIGKILL");
+    if (!killedGroup) {
+      try {
+        process.kill(processId, "SIGKILL");
+      } catch {
+        // Ignore process-kill failures during cleanup.
+      }
+    }
+    this.processTimeoutControls.delete(processControlKey);
+    this.liveProcessKeys.delete(processControlKey);
+  }
+
+  private getProcessIdFromControlKey(processControlKey: string): number | null {
+    const separatorIndex = processControlKey.lastIndexOf(":");
+    const rawProcessId = separatorIndex >= 0 ? processControlKey.slice(separatorIndex + 1) : processControlKey;
+    const processId = Number(rawProcessId);
+    return Number.isInteger(processId) && processId > 0 ? processId : null;
+  }
+
   private getProcessIds(processes: Map<string, SessionProcessEntry> | null): number[] {
     if (!processes) {
       return [];

src/tests/prompt.test.ts

@@ -48,7 +48,9 @@ test("getSystemPrompt includes UpdatePlan docs", () => {
 test("getSystemPrompt includes Bash background guidance", () => {
   const prompt = getSystemPrompt("/tmp/project");
   assert.equal(prompt.includes("run_in_background: true"), true);
-  assert.equal(prompt.includes("do not add `&`"), true);
+  assert.equal(prompt.includes("do NOT add `&`"), true);
+  assert.equal(prompt.includes("use the `stopCommand` returned in the tool result metadata"), true);
+  assert.equal(prompt.includes("stop background tasks that has not reported a completed state"), true);
 });
 
 test("getSystemPrompt does not include runtime context", () => {

src/tests/session.test.ts

@@ -108,6 +108,49 @@ test("SessionManager preserves structured system content when building OpenAI me
   ]);
 });
 
+test("SessionManager appends failed background log tail as XML", () => {
+  const workspace = createTempDir("deepcode-background-log-workspace-");
+  const home = createTempDir("deepcode-background-log-home-");
+  setHomeDir(home);
+  const outputPath = path.join(workspace, "background.log");
+  fs.writeFileSync(outputPath, ["before", "failure <line> & one", "failure line two"].join("\n"), "utf8");
+  let systemMessage: SessionMessage | null = null;
+  const manager = new SessionManager({
+    projectRoot: workspace,
+    createOpenAIClient: () => ({
+      client: null,
+      model: "test-model",
+      thinkingEnabled: false,
+    }),
+    getResolvedSettings: () => ({ model: "test-model" }),
+    renderMarkdown: (text) => text,
+    onAssistantMessage: (message) => {
+      systemMessage = message;
+    },
+  });
+
+  (manager as any).addBackgroundProcessCompletionMessage("session-background-fail", {
+    command: "npm test",
+    outputPath,
+    ok: false,
+    exitCode: 1,
+    signal: null,
+    startedAtMs: 0,
+    completedAtMs: 1200,
+  });
+
+  assert.ok(systemMessage);
+  const message = systemMessage as SessionMessage;
+  assert.equal(message.role, "system");
+  const content = message.content ?? "";
+  assert.match(content, /Background command "npm test" failed with exit code 1/);
+  assert.match(content, new RegExp(`<background_task_failure_log path="${escapeRegExp(outputPath)}">`));
+  assert.match(content, /failure <line> & one[\s\S]*failure line two/);
+  assert.doesNotMatch(content, /failure &lt;line&gt; &amp; one/);
+  assert.doesNotMatch(content, /<output_path>/);
+  assert.doesNotMatch(content, /<tail>/);
+});
+
 test("SessionManager filters image content for non-multimodal models", () => {
   const manager = new SessionManager({
     projectRoot: process.cwd(),
@@ -499,6 +542,67 @@ rl.on("line", (line) => {
   assert.deepEqual(manager.getMcpStatus(), []);
 });
 
+test("SessionManager dispose kills live processes without timeout controls", (t) => {
+  if (process.platform === "win32") {
+    t.skip("process group kill assertion is non-Windows specific");
+    return;
+  }
+
+  const workspace = createTempDir("deepcode-dispose-process-workspace-");
+  const home = createTempDir("deepcode-dispose-process-home-");
+  setHomeDir(home);
+  const manager = createSessionManager(workspace, "machine-id-dispose-process");
+  const sessionId = createSessionAndMessages(manager, "session-dispose-process", "Dispose process session");
+  const originalKill = process.kill;
+  const killed: Array<{ pid: number; signal?: NodeJS.Signals | number }> = [];
+
+  try {
+    process.kill = ((pid: number, signal?: NodeJS.Signals | number) => {
+      killed.push({ pid, signal });
+      return true;
+    }) as typeof process.kill;
+
+    (manager as any).addSessionProcess(sessionId, 1234, "python3 -m http.server 8080");
+    manager.dispose();
+  } finally {
+    process.kill = originalKill;
+  }
+
+  assert.deepEqual(killed, [{ pid: -1234, signal: "SIGKILL" }]);
+});
+
+test("SessionManager deleteSession ignores persisted processes that are not live", (t) => {
+  if (process.platform === "win32") {
+    t.skip("process group kill assertion is non-Windows specific");
+    return;
+  }
+
+  const workspace = createTempDir("deepcode-delete-stale-process-workspace-");
+  const home = createTempDir("deepcode-delete-stale-process-home-");
+  setHomeDir(home);
+  const manager = createSessionManager(workspace, "machine-id-delete-stale-process");
+  const sessionId = createSessionAndMessages(manager, "session-delete-stale-process", "Delete stale process session");
+  (manager as any).updateSessionEntry(sessionId, (entry: any) => ({
+    ...entry,
+    processes: new Map([["1234", { startTime: new Date().toISOString(), command: "stale process" }]]),
+  }));
+  const originalKill = process.kill;
+  const killed: Array<{ pid: number; signal?: NodeJS.Signals | number }> = [];
+
+  try {
+    process.kill = ((pid: number, signal?: NodeJS.Signals | number) => {
+      killed.push({ pid, signal });
+      return true;
+    }) as typeof process.kill;
+
+    assert.equal(manager.deleteSession(sessionId), true);
+  } finally {
+    process.kill = originalKill;
+  }
+
+  assert.deepEqual(killed, []);
+});
+
 test("SessionManager refreshes cached MCP tool definitions after server crash", async () => {
   const workspace = createTempDir("deepcode-mcp-crash-cache-workspace-");
   const serverPath = path.join(workspace, "mcp-server-crash.cjs");

src/tests/tool-handlers.test.ts

@@ -131,6 +131,13 @@ test("Bash can run commands in the background and report completion output", asy
   assert.equal(result.metadata?.runInBackground, true);
   assert.equal(typeof result.metadata?.backgroundTaskId, "string");
   assert.equal(typeof result.metadata?.outputPath, "string");
+  assert.equal(typeof result.metadata?.processId, "number");
+  const stopCommand =
+    process.platform === "win32"
+      ? `cmd.exe /c "taskkill /PID ${result.metadata.processId} /T /F"`
+      : `kill -- -${result.metadata.processId}`;
+  assert.equal(result.metadata?.stopCommand, stopCommand);
+  assert.match(result.output ?? "", /Stop it with:/);
   assert.ok(Date.now() - startedAt < 500);
   assert.equal(starts.length, 1);
 
@@ -176,6 +183,38 @@ test("Bash background completion reports failed exit codes", async () => {
   assert.match(output, /bad/);
 });
 
+test("Bash removes a trailing ampersand when run_in_background is true", async () => {
+  const workspace = createTempWorkspace();
+  let startedCommand = "";
+  let completion: BackgroundProcessCompletion | null = null;
+
+  const result = await handleBashTool(
+    {
+      command: "printf 'trimmed\\n' &",
+      run_in_background: true,
+    },
+    createContext("bash-background-trailing-ampersand", workspace, {
+      onProcessStart: (_pid, command) => {
+        startedCommand = command;
+      },
+      onBackgroundProcessComplete: (event) => {
+        completion = event;
+      },
+    })
+  );
+
+  assert.equal(result.ok, true);
+  assert.equal(startedCommand, "printf 'trimmed\\n'");
+
+  await waitFor(() => completion !== null, 2000);
+
+  assert.ok(completion);
+  const done = completion as BackgroundProcessCompletion;
+  assert.equal(done.command, "printf 'trimmed\\n'");
+  assert.equal(done.ok, true);
+  assert.equal(fs.readFileSync(done.outputPath, "utf8"), "trimmed\n");
+});
+
 test("UpdatePlan accepts a markdown task list string", async () => {
   const workspace = createTempWorkspace();
   const plan = ["## Task List", "", "- [>] Inspect current behavior", "- [ ] Implement UpdatePlan"].join("\n");