From 77c617f08be4ac816602f301120dadf88f0ea8c1 Mon Sep 17 00:00:00 2001 From: Thando Date: Wed, 24 Jun 2026 09:25:16 +0200 Subject: [PATCH] fix(conflict): reject resolver commentary instead of writing it to the file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port of the VXD fix. When the conflict-resolution model returns prose with NO fenced block ("Conflict resolved. Kept both sides…"), extractResolvedFileContent returns the prose and it gets written, destroying the file. looksLikeResolverChatter guards both the Senior and Tech-Lead paths so resolution fails (escalate/abort) instead of corrupting the file. Updated one test fixture whose placeholder "resolved content" tripped the new guard. Full suite green. --- internal/engine/conflict_extract_test.go | 31 ++++++++++++ internal/engine/conflict_resolver.go | 60 +++++++++++++++++++++++ internal/engine/conflict_resolver_test.go | 2 +- 3 files changed, 92 insertions(+), 1 deletion(-) diff --git a/internal/engine/conflict_extract_test.go b/internal/engine/conflict_extract_test.go index 4e01eb8..77ac67b 100644 --- a/internal/engine/conflict_extract_test.go +++ b/internal/engine/conflict_extract_test.go @@ -33,3 +33,34 @@ func TestExtractResolvedFileContent(t *testing.T) { t.Fatalf("fenced-only extraction failed: %q", g) } } + +func TestLooksLikeResolverChatter(t *testing.T) { + // Prose-only replies that destroy files when written verbatim (no fence to + // extract, so extraction returns the prose itself). + chatter := []string{ + "Conflict resolved. Kept both sides:\n\nNo selector collisions — separate class namespaces, all functionality retained.", + "Resolved file content:", + "Permission denied by harness. Cannot write the file myself. Resolved content below:", + "Working tree is `master` (no `src/`), so I can't write the file here.", + "Both sides merged — HEAD tests + branch tests. Write blocked on permission, so resolved content below:", + "Want me to apply this on branch nxd/abc and run the tests?", + } + for _, c := range chatter { + if !looksLikeResolverChatter(c) { + t.Errorf("expected chatter to be flagged:\n%q", c) + } + } + + // Real merged source must NOT be flagged. + code := []string{ + "package main\n\nfunc main() {}\n", + "import { useState } from 'react';\nexport const App = () => null;", + ".ui-button { color: red; }\n.app-layout { display: flex; }", + "{\n \"name\": \"frontend\",\n \"private\": true\n}", + } + for _, c := range code { + if looksLikeResolverChatter(c) { + t.Errorf("real source wrongly flagged as chatter:\n%q", c) + } + } +} diff --git a/internal/engine/conflict_resolver.go b/internal/engine/conflict_resolver.go index 1b81e15..9f9ece9 100644 --- a/internal/engine/conflict_resolver.go +++ b/internal/engine/conflict_resolver.go @@ -362,6 +362,12 @@ File: %s return "", fmt.Errorf("LLM output still contains conflict markers") } + // Reject conversational commentary. When the model returns prose with no + // fenced block, writing it would destroy the file; failing here escalates. + if looksLikeResolverChatter(resolved) { + return "", fmt.Errorf("LLM returned commentary, not file content") + } + return resolved, nil } @@ -437,6 +443,10 @@ CRITICAL OUTPUT RULES: return "", fmt.Errorf("tech lead output still contains conflict markers") } + if looksLikeResolverChatter(resolved) { + return "", fmt.Errorf("tech lead returned commentary, not file content") + } + return resolved, nil } @@ -528,6 +538,56 @@ func extractResolvedFileContent(resp string) string { return strings.TrimSpace(stripCodeFences(resp)) } +// resolverChatterMarkers are phrases that appear in a conflict-resolution model's +// CONVERSATIONAL reply but should never appear in actual merged source. When the +// model ignores the "return only file content" instruction and emits prose with +// NO fenced block (so extractResolvedFileContent has nothing to extract and +// returns the prose itself), writing that result DESTROYS the file. These markers +// are specific enough that real code/comments don't trip them. +var resolverChatterMarkers = []string{ + "conflict resolved", + "resolved content", + "resolved file content", + "resolved content below", + "resolved content:", + "kept both sides", + "both sides merged", + "kept head's", + "kept incoming", + "write blocked on permission", + "blocked on permission", + "permission denied by harness", + "cannot write the file", + "can't write the file", + "i cannot write", + "i can't write", + "can't write it here", + "want me to apply", + "want me to run", + "grant write to apply", + "working tree is", + "returning the resolved content", + "apply it to", + "apply this on branch", + "all functionality retained", + "no selector collisions", + "separate class namespaces", +} + +// looksLikeResolverChatter reports whether s is conflict-resolver commentary +// rather than merged file content. It is intentionally conservative: a single +// high-confidence marker is enough, because these phrases do not occur in valid +// source files, and writing chatter to disk corrupts the file irrecoverably. +func looksLikeResolverChatter(s string) bool { + lower := strings.ToLower(s) + for _, m := range resolverChatterMarkers { + if strings.Contains(lower, m) { + return true + } + } + return false +} + func stripCodeFences(s string) string { s = strings.TrimSpace(s) if strings.HasPrefix(s, "```") { diff --git a/internal/engine/conflict_resolver_test.go b/internal/engine/conflict_resolver_test.go index 7a25f3d..9c718d8 100644 --- a/internal/engine/conflict_resolver_test.go +++ b/internal/engine/conflict_resolver_test.go @@ -137,7 +137,7 @@ func TestResolveFile_PromptContainsNoFenceInstruction(t *testing.T) { // The ReplayClient returns a fixed response without recording the prompt, // so we verify the prompt by calling resolveFile with a controlled input // and checking there are no conflict markers in the output. - client := llm.NewReplayClient(llm.CompletionResponse{Content: "resolved content"}) + client := llm.NewReplayClient(llm.CompletionResponse{Content: "package main\n\nfunc main() {}\n"}) cr := &ConflictResolver{ llmClient: client,