From 2670bf3dacf18c30bfab76d15dc290ca47fac9ea Mon Sep 17 00:00:00 2001 From: yumakakuya Date: Thu, 7 May 2026 12:07:55 +0900 Subject: [PATCH 1/4] fix: expose session reopen tool --- .../java/dev/sorted/mcphub/McpHandler.java | 73 ++++++++++++++++++- .../dev/sorted/mcphub/McpHandlerTest.java | 34 ++++++++- 2 files changed, 100 insertions(+), 7 deletions(-) diff --git a/java/src/main/java/dev/sorted/mcphub/McpHandler.java b/java/src/main/java/dev/sorted/mcphub/McpHandler.java index f3da885..0867f65 100644 --- a/java/src/main/java/dev/sorted/mcphub/McpHandler.java +++ b/java/src/main/java/dev/sorted/mcphub/McpHandler.java @@ -34,6 +34,7 @@ public class McpHandler implements JsonRpcServer.MethodHandler { private static final Logger log = LoggerFactory.getLogger(McpHandler.class); private static final ObjectMapper mapper = new ObjectMapper(); private static final String DISAMBIGUATION_TOOL = "mcphub_disambiguate"; + private static final String SESSION_OPEN_TOOL = "mcphub.session.open"; private static final String SERVER_VERSION = "0.1.0-alpha"; private final StateMachine stateMachine; @@ -128,7 +129,12 @@ private JsonNode handleToolsList() { boolean isOpen = stateMachine.getState() == StateMachine.State.OPEN; if (!isOpen) { - // REQ-7.4.2: not Open → empty tool list + // Expose the recovery tool in the same AI-facing surface that reports session_not_open. + StateMachine.State current = stateMachine.getState(); + if (!stateMachine.isLockedUntilUnlock() + && (current == StateMachine.State.CLOSED || current == StateMachine.State.ARMED)) { + tools.add(buildSessionOpenTool()); + } r.set("tools", tools); return r; } @@ -202,6 +208,62 @@ private ObjectNode toMcpToolEntry(CapabilityEntry entry) { return tool; } + /** Build the MCP-visible session recovery tool. */ + private ObjectNode buildSessionOpenTool() { + ObjectNode tool = mapper.createObjectNode(); + tool.put("name", SESSION_OPEN_TOOL); + tool.put("description", "Open the MCPHUB session so tools become available. Call this when session is CLOSED or ARMED."); + ObjectNode schema = mapper.createObjectNode(); + schema.put("type", "object"); + schema.set("properties", mapper.createObjectNode()); + tool.set("inputSchema", schema); + return tool; + } + + /** Handle mcphub.session.open from tools/call so recovery is executable by AI clients. */ + private JsonNode handleSessionOpen(long startMs, int requestSizeBytes, String intentAnnotation) { + try { + StateMachine.State current = stateMachine.getState(); + String sessionId = sessionManager != null ? sessionManager.getCurrentSessionId() : null; + + if (current == StateMachine.State.OPEN) { + logRoute(sessionId, SESSION_OPEN_TOOL, "mcphub-internal", "builtin_hosted", + "allowed", null, System.currentTimeMillis() - startMs, + requestSizeBytes, 0, intentAnnotation, null); + ObjectNode resp = mapper.createObjectNode(); + resp.set("content", wrapTextContent("{\"state\":\"OPEN\",\"message\":\"Session already open.\"}")); + return resp; + } + + if (current == StateMachine.State.CLOSED) { + sessionId = sessionManager != null ? sessionManager.startSession() : "mcp-session"; + stateMachine.transition(StateMachine.Trigger.ARM, sessionId); + } else if (current == StateMachine.State.ARMED) { + sessionId = sessionManager != null ? sessionManager.getCurrentSessionId() : "mcp-session"; + } else { + return failureResponse("session_not_open", + "Cannot open session in state: " + current.name() + ". Wait and retry.", + "wait_session", null, null); + } + + stateMachine.transition(StateMachine.Trigger.OPEN, sessionId); + if (sessionManager != null) sessionManager.onOpen(); + + logRoute(sessionId, SESSION_OPEN_TOOL, "mcphub-internal", "builtin_hosted", + "allowed", null, System.currentTimeMillis() - startMs, + requestSizeBytes, 0, intentAnnotation, null); + + ObjectNode resp = mapper.createObjectNode(); + resp.put("mcphub_providers", "start"); + resp.set("content", wrapTextContent("{\"state\":\"OPEN\",\"session_id\":\"" + sessionId + "\",\"message\":\"Session opened. All tools are now available.\"}")); + return resp; + } catch (StateMachine.TransitionException e) { + return failureResponse("session_not_open", + "Failed to open session: " + e.getMessage(), + "retry", null, null); + } + } + /** Build the disambiguation MCP tool entry. REQ-5.3.1 */ private ObjectNode buildDisambiguationTool() { ObjectNode tool = mapper.createObjectNode(); @@ -269,6 +331,11 @@ private JsonNode handleToolsCall(JsonNode params) { return resp; } + // --- MCP-visible session recovery tool bypasses the state guard --- + if (SESSION_OPEN_TOOL.equals(toolName)) { + return handleSessionOpen(startMs, requestSizeBytes, intentAnnotation); + } + // --- State guard (REQ-2.4.5, REQ-5.6.2 session_not_open) --- if (stateMachine.getState() != StateMachine.State.OPEN) { long latency = System.currentTimeMillis() - startMs; @@ -276,8 +343,8 @@ private JsonNode handleToolsCall(JsonNode params) { "error", null, latency, requestSizeBytes, null, intentAnnotation, "session_not_open"); return failureResponse("session_not_open", - "Session is not Open. Call mcphub.control.open first.", - "wait_session", null, null); + "Session is not Open. Call mcphub.session.open to reopen the session.", + "call_mcphub_session_open", null, null); } // REQ-3.7.3: reset idle timer on every tools/call for the active session. diff --git a/java/src/test/java/dev/sorted/mcphub/McpHandlerTest.java b/java/src/test/java/dev/sorted/mcphub/McpHandlerTest.java index 266bbf0..73f7da9 100644 --- a/java/src/test/java/dev/sorted/mcphub/McpHandlerTest.java +++ b/java/src/test/java/dev/sorted/mcphub/McpHandlerTest.java @@ -71,10 +71,11 @@ void initialize_defaultServerName() throws Exception { // --- tools/list --- @Test - void toolsList_whenClosed_returnsEmpty() throws Exception { - // Session is CLOSED — tools/list must return empty list (REQ-7.4.2) + void toolsList_whenClosed_returnsSessionOpenTool() throws Exception { + // Session is CLOSED — recovery must be visible in the same AI-facing surface. JsonNode r = handler.handle("tools/list", null); - assertEquals(0, r.path("tools").size()); + assertEquals(1, r.path("tools").size()); + assertEquals("mcphub.session.open", r.path("tools").get(0).path("name").asText()); } @Test @@ -140,7 +141,32 @@ void toolsCall_sessionNotOpen_returnsSessionNotOpen() throws Exception { assertTrue(r.path("isError").asBoolean(), "isError must be true"); JsonNode err = parseErrorJson(r); assertEquals("session_not_open", err.path("error_code").asText()); - assertEquals("wait_session", err.path("next_action").asText()); + assertEquals("call_mcphub_session_open", err.path("next_action").asText()); + assertTrue(err.path("reason").asText().contains("mcphub.session.open")); + } + + @Test + void toolsCall_sessionOpenTool_opensSessionFromClosed() throws Exception { + ObjectNode params = mapper.createObjectNode(); + params.put("name", "mcphub.session.open"); + params.set("arguments", mapper.createObjectNode()); + + JsonNode r = handler.handle("tools/call", params); + + assertFalse(r.path("isError").asBoolean(false), "session recovery should not be an error"); + assertEquals(StateMachine.State.OPEN, sm.getState()); + assertEquals("start", r.path("mcphub_providers").asText()); + + JsonNode listed = handler.handle("tools/list", null); + JsonNode tools = listed.path("tools"); + boolean hasSessionOpen = false; + for (JsonNode t : tools) { + if ("mcphub.session.open".equals(t.path("name").asText())) { + hasSessionOpen = true; + break; + } + } + assertFalse(hasSessionOpen, "session recovery tool should not remain in the OPEN tool surface"); } @Test From 65162c5a529749473bea6dec2945b2b9f6242cb9 Mon Sep 17 00:00:00 2001 From: yumakakuya Date: Thu, 7 May 2026 12:21:53 +0900 Subject: [PATCH 2/4] fix: classify apply_patch failures --- adapters/edit/index.ts | 113 +++++++++++++++++++++++++++++++++++-- integration/vt_test.go | 125 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 234 insertions(+), 4 deletions(-) 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")) +} From 674edf7af83bd96f85c0a3b604128bb2cf4021ec Mon Sep 17 00:00:00 2001 From: yumakakuya Date: Thu, 7 May 2026 12:31:33 +0900 Subject: [PATCH 3/4] test: cover session reopen recovery --- integration/vt_test.go | 85 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/integration/vt_test.go b/integration/vt_test.go index 561ee17..73fd5ed 100644 --- a/integration/vt_test.go +++ b/integration/vt_test.go @@ -730,3 +730,88 @@ 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) +} From bb9303d2a7e49bfab7166198273663ef480438d9 Mon Sep 17 00:00:00 2001 From: yumakakuya Date: Thu, 7 May 2026 12:21:53 +0900 Subject: [PATCH 4/4] fix: classify apply_patch failures --- adapters/edit/index.ts | 113 +++++++++++++++++++++++++++++++++++-- integration/vt_test.go | 125 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 234 insertions(+), 4 deletions(-) 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 73fd5ed..51719da 100644 --- a/integration/vt_test.go +++ b/integration/vt_test.go @@ -815,3 +815,128 @@ func TestSessionOpenRecoveryTool(t *testing.T) { 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")) +}