fix: classify apply_patch failures

adapters/edit/index.ts

@@ -4,6 +4,109 @@ import * as os from 'os';
 import * as path from 'path';
 import * as child_process from 'child_process';
 
+type NextAction = 'retry' | 'use_alternative' | 'disambiguate' | 'abort' | 'wait_session' | 'call_mcphub_session_open';
+
+/** Check whether a patch uses non-git-style headers that will fail with patch -p1. */
+function hasNonGitStyleHeaders(patchContent: string): boolean {
+  for (const line of patchContent.split('\n')) {
+    if (line.startsWith('--- ')) {
+      const p = line.slice(4).trim();
+      if (p !== '/dev/null' && !p.startsWith('a/')) return true;
+    }
+    if (line.startsWith('+++ ')) {
+      const p = line.slice(4).trim();
+      if (p !== '/dev/null' && !p.startsWith('b/')) return true;
+    }
+  }
+  return false;
+}
+
+function formatPatchMessage(message: string, nextAction: NextAction): string {
+  return `[apply_patch] ${message} | next_action: ${nextAction}`;
+}
+
+/** Classify patch failure output into AI-actionable diagnostic text. */
+function classifyPatchFailure(
+  patchContent: string,
+  spawnErr: Error | null,
+  stdout: string,
+  stderr: string
+): string {
+  const output = (stderr + '\n' + stdout).trim();
+
+  if (spawnErr) {
+    const msg = spawnErr.message || '';
+    if (msg.includes('ETIMEDOUT') || msg.includes('timed out')) {
+      return formatPatchMessage(
+        'patch timed out (30s). The patch may be too large or the filesystem is slow. Try a smaller patch or check the target filesystem.',
+        'retry'
+      );
+    }
+    if (msg.includes('ENOENT')) {
+      return formatPatchMessage(
+        "patch binary not found on PATH. Install 'patch' (e.g. 'apt install patch' / 'brew install patchutils') and retry.",
+        'retry'
+      );
+    }
+    return formatPatchMessage(`patch spawn error: ${msg}`, 'retry');
+  }
+
+  if (hasNonGitStyleHeaders(patchContent)) {
+    return formatPatchMessage(
+      "Patch uses plain diff headers (e.g. '--- file' instead of '--- a/file'). " +
+      "The adapter applies patches with 'patch -p1' from supplied cwd - git-style 'a/' and 'b/' path prefixes are required. " +
+      "Re-generate the patch with 'git diff', 'git format-patch', or manually prefix paths with 'a/' and 'b/'.",
+      'abort'
+    );
+  }
+
+  if (output.includes("can't find file to patch")) {
+    return formatPatchMessage(
+      "Cannot find file to patch. Verify the target file exists relative to the working directory (cwd), " +
+      "or adjust the 'cwd' parameter. Ensure git-style headers ('--- a/path', '+++ b/path') are used.",
+      'disambiguate'
+    );
+  }
+
+  if (output.includes('File to patch:') || output.includes('Skip this patch?') || output.includes('Ignore this patch?')) {
+    return formatPatchMessage(
+      'Patch required interactive input. The file referenced in the patch may not exist, or the strip level (-p1) is wrong for the header format. Check paths relative to cwd and use git-style headers.',
+      'disambiguate'
+    );
+  }
+
+  const hunkFails = (output.match(/Hunk #\d+ FAILED/g) || []).length;
+  if (hunkFails > 0) {
+    return formatPatchMessage(
+      `${hunkFails} hunk(s) failed to apply. Patch may be stale - file content has changed since the patch was generated, or the context lines don't match. Re-generate the patch against the current file content.`,
+      'retry'
+    );
+  }
+
+  if (output.includes('reversed') || output.includes('previously applied') || output.includes('already applied')) {
+    return formatPatchMessage(
+      'Patch appears to be already applied (reversed or previously applied detected). Skip this patch or re-generate from a clean state.',
+      'abort'
+    );
+  }
+
+  if (output.includes('unexpectedly ends') || output.includes('malformed') || output.includes('Not a unified diff')) {
+    return formatPatchMessage(
+      "Patch content is malformed - not a valid unified diff. Verify the patch has correct '---', '+++', and '@@' headers.",
+      'abort'
+    );
+  }
+
+  if (!output) {
+    return formatPatchMessage(
+      'patch exited with non-zero status but produced no output. The patch may be invalid or the filesystem is in an unexpected state.',
+      'abort'
+    );
+  }
+
+  return formatPatchMessage(`patch failed: ${output.slice(0, 400)}`, 'abort');
+}
+
 const TOOLS = [
   {
     name: 'apply_patch',
@@ -24,14 +127,16 @@ async function dispatch(name: string, args: Record<string, unknown>): Promise<{
     const patch = args['patch'] as string;
     const cwd = (args['cwd'] as string) || process.cwd();
     if (!patch) throw new Error('patch is required');
-    // Write patch to temp file and apply
+    // Write patch to temp file and apply.
     const tmpFile = path.join(os.tmpdir(), `mcphub_patch_${Date.now()}.patch`);
     fs.writeFileSync(tmpFile, patch, 'utf8');
     try {
-      const result = child_process.spawnSync('patch', ['-p1', '--input', tmpFile], { cwd, encoding: 'utf8', timeout: 30000 });
+      // -f forces non-interactive behavior so the adapter returns diagnostics instead of waiting for input.
+      const result = child_process.spawnSync('patch', ['-p1', '-f', '--input', tmpFile], { cwd, encoding: 'utf8', timeout: 30000 });
       fs.unlinkSync(tmpFile);
-      if (result.status !== 0) {
-        return { content: [{ type: 'text', text: `patch failed:\n${result.stderr || result.stdout}` }], isError: true };
+      if (result.error || result.status !== 0) {
+        const text = classifyPatchFailure(patch, result.error ?? null, result.stdout ?? '', result.stderr ?? '');
+        return { content: [{ type: 'text', text }], isError: true };
       }
       return { content: [{ type: 'text', text: result.stdout || 'Patch applied successfully.' }] };
     } catch (e) {

integration/vt_test.go

@@ -730,3 +730,213 @@ func TestVT_020a_ArmedTimeoutAutoClose(t *testing.T) {
 		t.Errorf("VT-020a: expected CLOSED after armed timeout, got %s", state)
 	}
 }
+
+// TestSessionOpenRecoveryTool verifies the AI-facing recovery path for closed sessions.
+func TestSessionOpenRecoveryTool(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
+
+	repoRoot := findRepoRoot(t)
+	ensureJavaJar(t, repoRoot)
+	binPath := buildBinary(t, repoRoot)
+	sockPath, _, cleanup := startDaemon(t, binPath, repoRoot)
+	defer cleanup()
+
+	t.Run("closed_tools_list_shows_session_open_only", func(t *testing.T) {
+		result, rpcErr := call(t, sockPath, "tools/list", nil)
+		if rpcErr != nil {
+			t.Fatalf("tools/list RPC error: %d %s", rpcErr.Code, rpcErr.Message)
+		}
+		var listResult struct {
+			Tools []struct {
+				Name string `json:"name"`
+			} `json:"tools"`
+		}
+		if err := json.Unmarshal(result, &listResult); err != nil {
+			t.Fatalf("unmarshal tools/list: %v", err)
+		}
+		if len(listResult.Tools) != 1 {
+			t.Fatalf("expected 1 tool in CLOSED state, got %d", len(listResult.Tools))
+		}
+		if listResult.Tools[0].Name != "mcphub.session.open" {
+			t.Fatalf("expected mcphub.session.open, got %s", listResult.Tools[0].Name)
+		}
+	})
+
+	t.Run("closed_tool_call_returns_actionable_recovery", func(t *testing.T) {
+		params := map[string]interface{}{"name": "webfetch", "arguments": map[string]interface{}{"url": "https://example.com"}}
+		result, rpcErr := call(t, sockPath, "tools/call", params)
+		if rpcErr != nil {
+			t.Fatalf("tools/call RPC error: %d %s", rpcErr.Code, rpcErr.Message)
+		}
+		var out map[string]interface{}
+		if err := json.Unmarshal(result, &out); err != nil {
+			t.Fatalf("unmarshal result: %v", err)
+		}
+		if out["isError"] != true {
+			t.Fatalf("expected isError=true, got %v", out["isError"])
+		}
+		contentArr, ok := out["content"].([]interface{})
+		if !ok || len(contentArr) == 0 {
+			t.Fatalf("expected content array, got %v", out["content"])
+		}
+		textContent, ok := contentArr[0].(map[string]interface{})["text"].(string)
+		if !ok {
+			t.Fatalf("content[0].text not a string: %T", contentArr[0])
+		}
+		if !strings.Contains(textContent, "mcphub.session.open") {
+			t.Errorf("error must mention mcphub.session.open, got: %s", textContent)
+		}
+		if !strings.Contains(textContent, "call_mcphub_session_open") {
+			t.Errorf("error must include call_mcphub_session_open next_action, got: %s", textContent)
+		}
+	})
+
+	t.Run("session_open_recovers", func(t *testing.T) {
+		params := map[string]interface{}{"name": "mcphub.session.open", "arguments": map[string]interface{}{}}
+		result, rpcErr := call(t, sockPath, "tools/call", params)
+		if rpcErr != nil {
+			t.Fatalf("mcphub.session.open RPC error: %d %s", rpcErr.Code, rpcErr.Message)
+		}
+		var out map[string]interface{}
+		if err := json.Unmarshal(result, &out); err != nil {
+			t.Fatalf("unmarshal session.open result: %v", err)
+		}
+		if isError, _ := out["isError"].(bool); isError {
+			t.Fatalf("mcphub.session.open should not return an error: %v", out)
+		}
+	})
+
+	time.Sleep(2 * time.Second)
+	if got := statusState(t, sockPath); got != "OPEN" {
+		t.Fatalf("expected OPEN after recovery, got %s", got)
+	}
+
+	closeSession(t, sockPath)
+}
+
+// TestApplyPatch_Classifications verifies deterministic failure diagnostics for
+// apply_patch output. Covers: success with git-style headers, plain headers failure,
+// file-not-found, and hunk failures. No external services required.
+func TestApplyPatch_Classifications(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
+
+	repoRoot := findRepoRoot(t)
+	ensureJavaJar(t, repoRoot)
+	binPath := buildBinary(t, repoRoot)
+	sockPath, _, cleanup := startDaemon(t, binPath, repoRoot)
+	defer cleanup()
+
+	armAndOpen(t, sockPath)
+	defer closeSession(t, sockPath)
+
+	scratchDir := t.TempDir()
+
+	hunkFile := filepath.Join(scratchDir, "hunk_test.txt")
+	if err := os.WriteFile(hunkFile, []byte("line1\nline2\nline3\n"), 0644); err != nil {
+		t.Fatalf("write hunk_test.txt: %v", err)
+	}
+
+	type testCase struct {
+		name           string
+		patchContent   string
+		cwd            string
+		wantText       string
+		wantNextAction string
+		isError        bool
+	}
+
+	tests := []testCase{
+		{
+			name:           "success_git_style_headers",
+			patchContent:   "--- /dev/null\n+++ b/success_test.txt\n@@ -0,0 +1 @@\n+hello\n",
+			cwd:            scratchDir,
+			wantText:       "",
+			wantNextAction: "",
+			isError:        false,
+		},
+		{
+			name:           "plain_headers_detected",
+			patchContent:   "--- file.txt\n+++ file.txt\n@@ -1,1 +1,1 @@\n-old\n+new\n",
+			cwd:            scratchDir,
+			wantText:       "git-style",
+			wantNextAction: "abort",
+			isError:        true,
+		},
+		{
+			name:           "file_not_found",
+			patchContent:   "--- a/nonexistent.txt\n+++ b/nonexistent.txt\n@@ -1,1 +1,1 @@\n-old\n+new\n",
+			cwd:            scratchDir,
+			wantText:       "Cannot find file",
+			wantNextAction: "disambiguate",
+			isError:        true,
+		},
+		{
+			name:           "hunk_failure_stale_context",
+			patchContent:   "--- a/hunk_test.txt\n+++ b/hunk_test.txt\n@@ -1,3 +1,3 @@\n-wrong\n-line2\n-line3\n+correct\n+line2\n+line3\n",
+			cwd:            scratchDir,
+			wantText:       "hunk(s) failed",
+			wantNextAction: "retry",
+			isError:        true,
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			params := map[string]interface{}{
+				"name":      "apply_patch",
+				"arguments": map[string]interface{}{"patch": tc.patchContent, "cwd": tc.cwd},
+			}
+
+			result, rpcErr := call(t, sockPath, "tools/call", params)
+			if rpcErr != nil {
+				t.Errorf("RPC error: %d %s", rpcErr.Code, rpcErr.Message)
+				return
+			}
+
+			var out map[string]interface{}
+			if err := json.Unmarshal(result, &out); err != nil {
+				t.Errorf("unmarshal result: %v", err)
+				return
+			}
+
+			isError, _ := out["isError"].(bool)
+			if isError != tc.isError {
+				t.Errorf("isError: got %v, want %v", isError, tc.isError)
+			}
+
+			contentArr, ok := out["content"].([]interface{})
+			if !ok || len(contentArr) == 0 {
+				t.Errorf("expected content array, got %v", out)
+				return
+			}
+			textContent, ok := contentArr[0].(map[string]interface{})["text"].(string)
+			if !ok {
+				t.Errorf("content[0].text not a string: %T", contentArr[0])
+				return
+			}
+
+			if tc.wantText == "" && !tc.isError {
+				if !strings.Contains(strings.ToLower(textContent), "patch") && !strings.Contains(strings.ToLower(textContent), "applied") {
+					t.Errorf("expected success message containing 'patch' or 'applied', got: %s", textContent)
+				}
+			} else if tc.wantText != "" {
+				if !strings.Contains(textContent, tc.wantText) {
+					t.Errorf("content text does not contain %q, got: %s", tc.wantText, textContent)
+				}
+			}
+
+			if tc.wantNextAction != "" {
+				if !strings.Contains(textContent, "next_action: "+tc.wantNextAction) {
+					t.Errorf("next_action not %q in text, got: %s", tc.wantNextAction, textContent)
+				}
+			}
+		})
+	}
+
+	_ = os.Remove(filepath.Join(scratchDir, "success_test.txt"))
+}