fix(core): atomic session writes, MCP error classification, bg-log sweep

P1/P2 hardening on top of the private-settings fix:

1. Atomic session writes (session.ts)
   - saveSessionsIndex / saveSessionMessages now write via
     writeFileAtomic (tmp + rename) instead of writeFileSync, so a crash
     mid-write never leaves a truncated sessions-index.json / .jsonl.

2. MCP error classification (mcp-manager.ts)
   - classifyMcpError() tags tool failures as timeout/connection/auth/
     protocol/busy so the agent can distinguish a wedged server (retry)
     from a config error (don't retry). Surface as mcpErrorKind on the
     tool result.

3. Background log sweep (bash-handler.ts)
   - sweepOldBackgroundLogs() removes deepcode-background/*.log older than
     7 days on each background start (was never cleaned).

Evaluated and left as-is: sessionWorkingDirs is already cleared on session
delete; deps already use package-lock.json + npm ci in CI.

Tests: hardening.test.ts (atomic write, classifyMcpError, sweep) — 9 new
cases. Full suite: 293 tests, 286 pass, 0 fail. tsc --noEmit clean.

Author: raymondginger2018-sudo

packages/core/src/common/private-storage.ts

@@ -108,3 +108,30 @@ export function ensurePrivateDirectory(dirPath: string): void {
 export function deepcodeHome(): string {
   return path.join(os.homedir(), ".deepcode");
 }
+
+/**
+ * Atomically write ``contents`` to ``targetPath`` with user-only permissions.
+ *
+ * Writes to a sibling ``.tmp`` file first, then ``fs.rename`` over the target.
+ * A crash mid-write leaves the previous content intact instead of a truncated
+ * file. On Windows, the private ACL is applied to the temp file *before* the
+ * rename so the final path never exists with a permissive ACL.
+ */
+export function writeFileAtomic(targetPath: string, contents: string): void {
+  const tmpPath = `${targetPath}.tmp`;
+  writePrivateFile(tmpPath, contents);
+  try {
+    fs.renameSync(tmpPath, targetPath);
+  } catch (err) {
+    try {
+      fs.unlinkSync(tmpPath);
+    } catch {
+      /* ignore cleanup failure */
+    }
+    throw err;
+  }
+  if (process.platform === "win32") {
+    // After rename the final path may carry a new (inherited) ACL; re-restrict.
+    restrictWindowsAcl(targetPath);
+  }
+}

packages/core/src/mcp/mcp-manager.ts

@@ -6,9 +6,37 @@ const MCP_STARTUP_TIMEOUT_MS = process.env.DEEPCODE_MCP_TIMEOUT
   ? parseInt(process.env.DEEPCODE_MCP_TIMEOUT, 10)
   : 30_000;
 const MCP_CALL_TOOL_TIMEOUT_MS = 60_000;
+/** Connection-establishment budget for MCP servers (startup + handshake). */
+const MCP_CONNECT_TIMEOUT_MS = 15_000;
 const API_TOOL_NAME_PATTERN = /^[a-zA-Z0-9_-]+$/;
 const API_TOOL_NAME_MAX_LENGTH = 64;
 
+/**
+ * Classify an MCP error message so the agent can respond appropriately:
+ * a timed-out server should be retried / restarted, a config error should
+ * not be retried. Mirrors the failure taxonomy used by the MCP reference
+ * implementations (connection / protocol / timeout / auth / busy).
+ */
+export function classifyMcpError(message: string): "timeout" | "connection" | "protocol" | "auth" | "busy" | "unknown" {
+  const m = message.toLowerCase();
+  if (/timed out|timeout|deadline/.test(m)) {
+    return "timeout";
+  }
+  if (/connection refused|connect (call )?failed|failed to start|no such file|spawn .* enoent|not found/.test(m)) {
+    return "connection";
+  }
+  if (/unauthorized|forbidden|401|403|authentication|invalid api|api ?key/.test(m)) {
+    return "auth";
+  }
+  if (/parse error|invalid json|protocol|schema|unsupported/.test(m)) {
+    return "protocol";
+  }
+  if (/busy|locked|in progress/.test(m)) {
+    return "busy";
+  }
+  return "unknown";
+}
+
 type McpToolEntry = {
   serverName: string;
   originalName: string;
@@ -359,10 +387,16 @@ export class McpManager {
         output: text || JSON.stringify(result.content),
       };
     } catch (err) {
+      const message = err instanceof Error ? err.message : String(err);
+      const kind = classifyMcpError(message);
       return {
         ok: false,
         name,
-        error: err instanceof Error ? err.message : String(err),
+        error: message,
+        // Structured classification lets the agent distinguish a wedged
+        // server (timeout) from a config error (auth/protocol) instead of
+        // retrying blindly.
+        ...(kind !== "unknown" ? { mcpErrorKind: kind } : {}),
       };
     }
   }

packages/core/src/session.ts

@@ -35,6 +35,7 @@ import {
   type PermissionSettings,
 } from "./settings";
 import { logApiError } from "./common/error-logger";
+import { writeFileAtomic } from "./common/private-storage";
 import { logOpenAIChatCompletionDebug, normalizeDebugError } from "./common/debug-logger";
 import { describeLlmError, getLlmErrorDetails } from "./common/llm-error";
 import { killProcessTree } from "./common/process-tree";
@@ -2126,7 +2127,7 @@ ${agentInstructions}
       })),
       originalPath: this.projectRoot,
     };
-    fs.writeFileSync(sessionsIndexPath, JSON.stringify(normalized, null, 2), "utf8");
+    writeFileAtomic(sessionsIndexPath, JSON.stringify(normalized, null, 2));
   }
 
   private getSessionMessagesPath(sessionId: string): string {
@@ -2197,7 +2198,7 @@ ${agentInstructions}
     this.ensureProjectDir();
     const messagePath = this.getSessionMessagesPath(sessionId);
     const payload = messages.map((message) => JSON.stringify(message)).join("\n");
-    fs.writeFileSync(messagePath, payload ? `${payload}\n` : "", "utf8");
+    writeFileAtomic(messagePath, payload ? `${payload}\n` : "");
   }
 
   private updateSessionEntry(sessionId: string, updater: (entry: SessionEntry) => SessionEntry): SessionEntry | null {

packages/core/src/tests/hardening.test.ts

@@ -0,0 +1,99 @@
+import { afterEach, describe, it } from "node:test";
+import assert from "node:assert/strict";
+import * as fs from "fs";
+import * as os from "os";
+import * as path from "path";
+import { writeFileAtomic } from "../common/private-storage";
+import { classifyMcpError } from "../mcp/mcp-manager";
+import { sweepOldBackgroundLogs } from "../tools/bash-handler";
+
+const tempDirs: string[] = [];
+
+function tempDir(prefix: string): string {
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix));
+  tempDirs.push(dir);
+  return dir;
+}
+
+afterEach(() => {
+  for (const dir of tempDirs.splice(0)) {
+    fs.rmSync(dir, { recursive: true, force: true });
+  }
+});
+
+describe("writeFileAtomic", () => {
+  it("replaces the target and leaves no .tmp behind", () => {
+    const dir = tempDir("dc-atomic-");
+    const target = path.join(dir, "index.json");
+    fs.writeFileSync(target, "old", "utf8");
+    writeFileAtomic(target, "new");
+    assert.equal(fs.readFileSync(target, "utf8"), "new");
+    assert.ok(!fs.existsSync(`${target}.tmp`), "tmp file must be cleaned up");
+  });
+
+  it("writes 0600 on POSIX", () => {
+    if (process.platform === "win32") {
+      return;
+    }
+    const dir = tempDir("dc-atomic-mode-");
+    const target = path.join(dir, "s.json");
+    writeFileAtomic(target, "{}");
+    const mode = fs.statSync(target).mode & 0o777;
+    assert.equal(mode, 0o600);
+  });
+});
+
+describe("classifyMcpError", () => {
+  it("classifies timeouts", () => {
+    assert.equal(classifyMcpError("Timed out after 60000ms"), "timeout");
+    assert.equal(classifyMcpError("deadline exceeded"), "timeout");
+  });
+
+  it("classifies connection failures", () => {
+    assert.equal(classifyMcpError("Failed to start MCP server: ENOENT"), "connection");
+    assert.equal(classifyMcpError("connection refused"), "connection");
+  });
+
+  it("classifies auth failures", () => {
+    assert.equal(classifyMcpError("HTTP 401 unauthorized"), "auth");
+    assert.equal(classifyMcpError("invalid api key"), "auth");
+  });
+
+  it("classifies protocol and busy", () => {
+    assert.equal(classifyMcpError("invalid json payload"), "protocol");
+    assert.equal(classifyMcpError("server busy, retry later"), "busy");
+  });
+
+  it("returns unknown otherwise", () => {
+    assert.equal(classifyMcpError("something unexpected"), "unknown");
+  });
+});
+
+describe("sweepOldBackgroundLogs", () => {
+  it("deletes only expired .log files", () => {
+    const dir = tempDir("dc-sweep-");
+
+    const oldLog = path.join(dir, "old.log");
+    const freshLog = path.join(dir, "fresh.log");
+    const otherFile = path.join(dir, "keep.txt");
+    fs.writeFileSync(oldLog, "old");
+    fs.writeFileSync(freshLog, "fresh");
+    fs.writeFileSync(otherFile, "keep");
+
+    // Simulate old file: 8 days ago; fresh: now.
+    const now = Date.now();
+    const eightDaysAgo = now - 8 * 24 * 60 * 60 * 1000;
+    fs.utimesSync(oldLog, new Date(eightDaysAgo), new Date(eightDaysAgo));
+
+    sweepOldBackgroundLogs(now, dir);
+
+    assert.ok(!fs.existsSync(oldLog), "expired .log must be deleted");
+    assert.ok(fs.existsSync(freshLog), "fresh .log must be kept");
+    assert.ok(fs.existsSync(otherFile), "non-log file must be kept");
+  });
+
+  it("is a no-op when the directory does not exist", () => {
+    const missing = path.join(os.tmpdir(), "dc-no-such-sweep-dir");
+    assert.doesNotThrow(() => sweepOldBackgroundLogs(Date.now(), missing));
+  });
+});

packages/core/src/tools/bash-handler.ts

@@ -18,6 +18,8 @@ import {
 const MAX_OUTPUT_CHARS = 30000;
 const MAX_CAPTURE_CHARS = 10 * 1024 * 1024;
 const BACKGROUND_OUTPUT_DIR = path.join(os.tmpdir(), "deepcode-background");
+/** Background-task logs older than this are deleted on the next background start. */
+const BACKGROUND_LOG_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000;
 const TRAILING_BACKGROUND_OPERATOR_PATTERN = /(^|[^\\&])\s*&\s*$/;
 const sessionWorkingDirs = new Map<string, string>();
 
@@ -28,6 +30,37 @@ export function clearSessionWorkingDir(sessionId: string): void {
   sessionWorkingDirs.delete(sessionId);
 }
 
+/**
+ * Delete background-task logs older than {@link BACKGROUND_LOG_MAX_AGE_MS}.
+ * Best-effort: the temp dir is shared and may contain files from other
+ * processes, so we only remove ``*.log`` files past the age cutoff and never
+ * touch directories. Called once per background command start — cheap enough
+ * to avoid a dedicated sweeper task.
+ */
+export function sweepOldBackgroundLogs(nowMs: number = Date.now(), dir: string = BACKGROUND_OUTPUT_DIR): void {
+  let entries: fs.Dirent[];
+  try {
+    entries = fs.readdirSync(dir, { withFileTypes: true });
+  } catch {
+    return; // dir does not exist yet
+  }
+  const cutoff = nowMs - BACKGROUND_LOG_MAX_AGE_MS;
+  for (const entry of entries) {
+    if (!entry.isFile() || !entry.name.endsWith(".log")) {
+      continue;
+    }
+    const full = path.join(dir, entry.name);
+    try {
+      const stat = fs.statSync(full);
+      if (stat.mtimeMs < cutoff) {
+        fs.unlinkSync(full);
+      }
+    } catch {
+      // File may have been removed concurrently; skip.
+    }
+  }
+}
+
 type ToolCommandResult = {
   ok: boolean;
   output: string;
@@ -264,6 +297,7 @@ function startBackgroundShellCommand(
   context: ToolExecutionContext
 ): ToolExecutionResult {
   fs.mkdirSync(BACKGROUND_OUTPUT_DIR, { recursive: true });
+  sweepOldBackgroundLogs();
   const taskId = `bash-${randomUUID()}`;
   const outputPath = path.join(BACKGROUND_OUTPUT_DIR, `${taskId}.log`);
   const startedAtMs = Date.now();