diff --git a/adapters/edit/index.ts b/adapters/edit/index.ts index 33aecc7..dcd692a 100644 --- a/adapters/edit/index.ts +++ b/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): 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) { diff --git a/integration/vt_test.go b/integration/vt_test.go index 561ee17..64dd82e 100644 --- a/integration/vt_test.go +++ b/integration/vt_test.go @@ -730,3 +730,128 @@ func TestVT_020a_ArmedTimeoutAutoClose(t *testing.T) { t.Errorf("VT-020a: expected CLOSED after armed timeout, got %s", state) } } + +// 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")) +}