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,