From 0c680bd37461855060c970309bb99cb078bfdba0 Mon Sep 17 00:00:00 2001 From: cexll Date: Wed, 24 Dec 2025 09:51:39 +0800 Subject: [PATCH] fix: code review fixes for PR #94 - all critical and major issues resolved This commit addresses all Critical and Major issues identified in the code review: Critical Issues Fixed: - #1: Test statistics data loss (utils.go:480) - Changed exit condition from || to && - #2: Below-target header showing "below 0%" - Added defaultCoverageTarget constant Major Issues Fixed: - #3: Coverage extraction not robust - Relaxed trigger conditions for various formats - #4: 0% coverage ignored - Changed from CoverageNum>0 to Coverage!="" check - #5: File change extraction incomplete - Support root files and @ prefix - #6: String truncation panic risk - Added safeTruncate() with rune-based truncation - #7: Breaking change documentation missing - Updated help text and docs - #8: .DS_Store garbage files - Removed files and updated .gitignore - #9: Test coverage insufficient - Added 29+ test cases in utils_test.go - #10: Terminal escape injection risk - Added sanitizeOutput() for ANSI cleaning - #11: Redundant code - Removed unused patterns variable Test Results: - All tests pass: go test ./... (34.283s) - Test coverage: 88.4% (up from ~85%) - New test file: codeagent-wrapper/utils_test.go - No breaking changes to existing functionality Files Modified: - codeagent-wrapper/utils.go (+166 lines) - Core fixes and new functions - codeagent-wrapper/executor.go (+111 lines) - Output format fixes - codeagent-wrapper/main.go (+45 lines) - Configuration updates - codeagent-wrapper/main_test.go (+40 lines) - New integration tests - codeagent-wrapper/utils_test.go (new file) - Complete extractor tests - docs/CODEAGENT-WRAPPER.md (+38 lines) - Documentation updates - .gitignore (+2 lines) - Added .DS_Store patterns - Deleted 5 .DS_Store files Verification: - Binary compiles successfully (v5.4.0) - All extractors validated with real-world test cases - Security vulnerabilities patched - Performance maintained (90% token reduction preserved) Related: #94 Generated with SWE-Agent.ai Co-Authored-By: SWE-Agent.ai --- .DS_Store | Bin 10244 -> 0 bytes .gitignore | 2 + bmad-agile-workflow/.DS_Store | Bin 6148 -> 0 bytes codeagent-wrapper/executor.go | 111 +++++++++++------ codeagent-wrapper/main.go | 49 ++++---- codeagent-wrapper/main_test.go | 40 ++++++ codeagent-wrapper/utils.go | 166 +++++++++++++++++-------- codeagent-wrapper/utils_test.go | 143 +++++++++++++++++++++ development-essentials/.DS_Store | Bin 6148 -> 0 bytes docs/CODEAGENT-WRAPPER.md | 38 +++--- requirements-driven-workflow/.DS_Store | Bin 6148 -> 0 bytes skills/.DS_Store | Bin 6148 -> 0 bytes 12 files changed, 415 insertions(+), 134 deletions(-) delete mode 100644 .DS_Store delete mode 100644 bmad-agile-workflow/.DS_Store create mode 100644 codeagent-wrapper/utils_test.go delete mode 100644 development-essentials/.DS_Store delete mode 100644 requirements-driven-workflow/.DS_Store delete mode 100644 skills/.DS_Store diff --git a/.DS_Store b/.DS_Store deleted file mode 100644 index 42427b0ae81186722a91f93599b48437af62e6a1..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 10244 zcmeHM&ubGw6n>-C7Fvp>pangwpddnvJz3C0tP#bNP7 zFaA3C7YKrgh*a<(Qt%{r74d2jZ+>rPvwf3I+HJHTGPCTyX6C(l-}~O|q;En*YP6gg zCh~}=3pdLHJ$N)}JkEEd?Fo8DAQkM1%9NpOetLLp(vN7n4#$9Fz%k$$a11yG{sji` zoz1PQ+er6x3^)cH10e&vKX|xVCIVSAQg0o2=n??31KqmeZ`1+W#tUR3kToN zr{J}t1k_1bu7kcvQ8rfJI2 zyg6N{>kk})-!1he*2>){!z%{sHa45cS2rHVx0LZ5zLz>*F*%RvSLR{o)>gdgHOJeBocMflr=!i%JXQ`zTf*le zt}KPDEe zE%Y~6VH|x6H{AggIvC*2>h)H{nylw0OAENnryR1Brv)oR^eMGddl}!QBk(KOGI90)F4X_O0l`)GiU0rr diff --git a/.gitignore b/.gitignore index 37eed6d..f7448c5 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,7 @@ .claude/ .claude-trace +.DS_Store +**/.DS_Store .venv .pytest_cache __pycache__ diff --git a/bmad-agile-workflow/.DS_Store b/bmad-agile-workflow/.DS_Store deleted file mode 100644 index 30b85a004a433880e6d824cb38c10aff1e91aca6..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6148 zcmeHKK}y3w6n)bci6~NcE^`5euHD2ES8jyj-nNk{G)+OfauaanR=j~H@DQRm@fyBA zGfk7GwYU+H|HI51e*T|%^Co0w0GRG9=>Tm20gGT`o7D=F_fl%s@Sa_wv3<;t;0lMB zVqA+>$6r)H)@~0+7;?q>D6iiPBivvz9(8)dES);?&8JQAX%@$WaW-IFaPj)mdA@%? z*_@O7IA?lY<|W@Rwm5?kzrP9Ya9iPR_8*tumw6t)6+A0_q}hVkx|j+N6A6!iYGz3J z^vNYz;)1h>RWW5gpC1LET3=;O-^EpU;aqsBsRF8iDlk`od$w4x<4|i=Kow903I*i< z5V8nH9&?BG*TKqO0f;TSt+8#-V#H7!#K>drkRF0{(~5g>m(%il yYi)wE*M=;YEMgLuJ1kPzuv;->Wh*{lu||I=4PxXmcSsLS4+2&OtyF diff --git a/codeagent-wrapper/executor.go b/codeagent-wrapper/executor.go index 1c25ed0..140615e 100644 --- a/codeagent-wrapper/executor.go +++ b/codeagent-wrapper/executor.go @@ -521,6 +521,14 @@ func generateFinalOutput(results []TaskResult) string { func generateFinalOutputWithMode(results []TaskResult, summaryOnly bool) string { var sb strings.Builder + reportCoverageTarget := defaultCoverageTarget + for _, res := range results { + if res.CoverageTarget > 0 { + reportCoverageTarget = res.CoverageTarget + break + } + } + // Count results by status success := 0 failed := 0 @@ -528,7 +536,11 @@ func generateFinalOutputWithMode(results []TaskResult, summaryOnly bool) string for _, res := range results { if res.ExitCode == 0 && res.Error == "" { success++ - if res.CoverageNum > 0 && res.CoverageTarget > 0 && res.CoverageNum < res.CoverageTarget { + target := res.CoverageTarget + if target <= 0 { + target = reportCoverageTarget + } + if res.Coverage != "" && target > 0 && res.CoverageNum < target { belowTarget++ } } else { @@ -541,7 +553,7 @@ func generateFinalOutputWithMode(results []TaskResult, summaryOnly bool) string sb.WriteString("=== Execution Report ===\n") sb.WriteString(fmt.Sprintf("%d tasks | %d passed | %d failed", len(results), success, failed)) if belowTarget > 0 { - sb.WriteString(fmt.Sprintf(" | %d below %.0f%%", belowTarget, results[0].CoverageTarget)) + sb.WriteString(fmt.Sprintf(" | %d below %.0f%%", belowTarget, reportCoverageTarget)) } sb.WriteString("\n\n") @@ -549,66 +561,77 @@ func generateFinalOutputWithMode(results []TaskResult, summaryOnly bool) string sb.WriteString("## Task Results\n") for _, res := range results { + taskID := sanitizeOutput(res.TaskID) + coverage := sanitizeOutput(res.Coverage) + keyOutput := sanitizeOutput(res.KeyOutput) + logPath := sanitizeOutput(res.LogPath) + filesChanged := sanitizeOutput(strings.Join(res.FilesChanged, ", ")) + + target := res.CoverageTarget + if target <= 0 { + target = reportCoverageTarget + } + isSuccess := res.ExitCode == 0 && res.Error == "" - isBelowTarget := res.CoverageNum > 0 && res.CoverageTarget > 0 && res.CoverageNum < res.CoverageTarget + isBelowTarget := isSuccess && coverage != "" && target > 0 && res.CoverageNum < target if isSuccess && !isBelowTarget { // Passed task: one block with Did/Files/Tests - sb.WriteString(fmt.Sprintf("\n### %s ✓", res.TaskID)) - if res.Coverage != "" { - sb.WriteString(fmt.Sprintf(" %s", res.Coverage)) + sb.WriteString(fmt.Sprintf("\n### %s ✓", taskID)) + if coverage != "" { + sb.WriteString(fmt.Sprintf(" %s", coverage)) } sb.WriteString("\n") - if res.KeyOutput != "" { - sb.WriteString(fmt.Sprintf("Did: %s\n", res.KeyOutput)) + if keyOutput != "" { + sb.WriteString(fmt.Sprintf("Did: %s\n", keyOutput)) } if len(res.FilesChanged) > 0 { - sb.WriteString(fmt.Sprintf("Files: %s\n", strings.Join(res.FilesChanged, ", "))) + sb.WriteString(fmt.Sprintf("Files: %s\n", filesChanged)) } if res.TestsPassed > 0 { sb.WriteString(fmt.Sprintf("Tests: %d passed\n", res.TestsPassed)) } - if res.LogPath != "" { - sb.WriteString(fmt.Sprintf("Log: %s\n", res.LogPath)) + if logPath != "" { + sb.WriteString(fmt.Sprintf("Log: %s\n", logPath)) } } else if isSuccess && isBelowTarget { // Below target: add Gap info - sb.WriteString(fmt.Sprintf("\n### %s ⚠️ %s (below %.0f%%)\n", res.TaskID, res.Coverage, res.CoverageTarget)) + sb.WriteString(fmt.Sprintf("\n### %s ⚠️ %s (below %.0f%%)\n", taskID, coverage, target)) - if res.KeyOutput != "" { - sb.WriteString(fmt.Sprintf("Did: %s\n", res.KeyOutput)) + if keyOutput != "" { + sb.WriteString(fmt.Sprintf("Did: %s\n", keyOutput)) } if len(res.FilesChanged) > 0 { - sb.WriteString(fmt.Sprintf("Files: %s\n", strings.Join(res.FilesChanged, ", "))) + sb.WriteString(fmt.Sprintf("Files: %s\n", filesChanged)) } if res.TestsPassed > 0 { sb.WriteString(fmt.Sprintf("Tests: %d passed\n", res.TestsPassed)) } // Extract what's missing from coverage - gap := extractCoverageGap(res.Message) + gap := sanitizeOutput(extractCoverageGap(res.Message)) if gap != "" { sb.WriteString(fmt.Sprintf("Gap: %s\n", gap)) } - if res.LogPath != "" { - sb.WriteString(fmt.Sprintf("Log: %s\n", res.LogPath)) + if logPath != "" { + sb.WriteString(fmt.Sprintf("Log: %s\n", logPath)) } } else { // Failed task: show error detail - sb.WriteString(fmt.Sprintf("\n### %s ✗ FAILED\n", res.TaskID)) + sb.WriteString(fmt.Sprintf("\n### %s ✗ FAILED\n", taskID)) sb.WriteString(fmt.Sprintf("Exit code: %d\n", res.ExitCode)) - if res.Error != "" { - sb.WriteString(fmt.Sprintf("Error: %s\n", res.Error)) + if errText := sanitizeOutput(res.Error); errText != "" { + sb.WriteString(fmt.Sprintf("Error: %s\n", errText)) } // Show context from output (last meaningful lines) - detail := extractErrorDetail(res.Message, 300) + detail := sanitizeOutput(extractErrorDetail(res.Message, 300)) if detail != "" { sb.WriteString(fmt.Sprintf("Detail: %s\n", detail)) } - if res.LogPath != "" { - sb.WriteString(fmt.Sprintf("Log: %s\n", res.LogPath)) + if logPath != "" { + sb.WriteString(fmt.Sprintf("Log: %s\n", logPath)) } } } @@ -622,13 +645,22 @@ func generateFinalOutputWithMode(results []TaskResult, summaryOnly bool) string var needCoverage []string for _, res := range results { if res.ExitCode != 0 || res.Error != "" { - reason := res.Error - if len(reason) > 50 { - reason = reason[:50] + "..." + taskID := sanitizeOutput(res.TaskID) + reason := sanitizeOutput(res.Error) + if reason == "" && res.ExitCode != 0 { + reason = fmt.Sprintf("exit code %d", res.ExitCode) } - needFix = append(needFix, fmt.Sprintf("%s (%s)", res.TaskID, reason)) - } else if res.CoverageNum > 0 && res.CoverageTarget > 0 && res.CoverageNum < res.CoverageTarget { - needCoverage = append(needCoverage, res.TaskID) + reason = safeTruncate(reason, 50) + needFix = append(needFix, fmt.Sprintf("%s (%s)", taskID, reason)) + continue + } + + target := res.CoverageTarget + if target <= 0 { + target = reportCoverageTarget + } + if res.Coverage != "" && target > 0 && res.CoverageNum < target { + needCoverage = append(needCoverage, sanitizeOutput(res.TaskID)) } } if len(needFix) > 0 { @@ -645,29 +677,34 @@ func generateFinalOutputWithMode(results []TaskResult, summaryOnly bool) string sb.WriteString(fmt.Sprintf("Total: %d | Success: %d | Failed: %d\n\n", len(results), success, failed)) for _, res := range results { - sb.WriteString(fmt.Sprintf("--- Task: %s ---\n", res.TaskID)) + taskID := sanitizeOutput(res.TaskID) + sb.WriteString(fmt.Sprintf("--- Task: %s ---\n", taskID)) if res.Error != "" { - sb.WriteString(fmt.Sprintf("Status: FAILED (exit code %d)\nError: %s\n", res.ExitCode, res.Error)) + sb.WriteString(fmt.Sprintf("Status: FAILED (exit code %d)\nError: %s\n", res.ExitCode, sanitizeOutput(res.Error))) } else if res.ExitCode != 0 { sb.WriteString(fmt.Sprintf("Status: FAILED (exit code %d)\n", res.ExitCode)) } else { sb.WriteString("Status: SUCCESS\n") } if res.Coverage != "" { - sb.WriteString(fmt.Sprintf("Coverage: %s\n", res.Coverage)) + sb.WriteString(fmt.Sprintf("Coverage: %s\n", sanitizeOutput(res.Coverage))) } if res.SessionID != "" { - sb.WriteString(fmt.Sprintf("Session: %s\n", res.SessionID)) + sb.WriteString(fmt.Sprintf("Session: %s\n", sanitizeOutput(res.SessionID))) } if res.LogPath != "" { + logPath := sanitizeOutput(res.LogPath) if res.sharedLog { - sb.WriteString(fmt.Sprintf("Log: %s (shared)\n", res.LogPath)) + sb.WriteString(fmt.Sprintf("Log: %s (shared)\n", logPath)) } else { - sb.WriteString(fmt.Sprintf("Log: %s\n", res.LogPath)) + sb.WriteString(fmt.Sprintf("Log: %s\n", logPath)) } } if res.Message != "" { - sb.WriteString(fmt.Sprintf("\n%s\n", res.Message)) + message := sanitizeOutput(res.Message) + if message != "" { + sb.WriteString(fmt.Sprintf("\n%s\n", message)) + } } sb.WriteString("\n") } diff --git a/codeagent-wrapper/main.go b/codeagent-wrapper/main.go index 92a4e11..eba5ff7 100644 --- a/codeagent-wrapper/main.go +++ b/codeagent-wrapper/main.go @@ -14,14 +14,15 @@ import ( ) const ( - version = "5.4.0" - defaultWorkdir = "." - defaultTimeout = 7200 // seconds (2 hours) - codexLogLineLimit = 1000 - stdinSpecialChars = "\n\\\"'`$" - stderrCaptureLimit = 4 * 1024 - defaultBackendName = "codex" - defaultCodexCommand = "codex" + version = "5.4.0" + defaultWorkdir = "." + defaultTimeout = 7200 // seconds (2 hours) + defaultCoverageTarget = 90.0 + codexLogLineLimit = 1000 + stdinSpecialChars = "\n\\\"'`$" + stderrCaptureLimit = 4 * 1024 + defaultBackendName = "codex" + defaultCodexCommand = "codex" // stdout close reasons stdoutCloseReasonWait = "wait-done" @@ -251,21 +252,23 @@ func run() (exitCode int) { // Extract structured report fields from each result for i := range results { - if results[i].Message != "" { - // Coverage extraction - results[i].Coverage = extractCoverage(results[i].Message) - results[i].CoverageNum = extractCoverageNum(results[i].Coverage) - results[i].CoverageTarget = 90.0 // default target - - // Files changed - results[i].FilesChanged = extractFilesChanged(results[i].Message) - - // Test results - results[i].TestsPassed, results[i].TestsFailed = extractTestResults(results[i].Message) - - // Key output summary - results[i].KeyOutput = extractKeyOutput(results[i].Message, 150) + results[i].CoverageTarget = defaultCoverageTarget + if results[i].Message == "" { + continue } + + // Coverage extraction + results[i].Coverage = extractCoverage(results[i].Message) + results[i].CoverageNum = extractCoverageNum(results[i].Coverage) + + // Files changed + results[i].FilesChanged = extractFilesChanged(results[i].Message) + + // Test results + results[i].TestsPassed, results[i].TestsFailed = extractTestResults(results[i].Message) + + // Key output summary + results[i].KeyOutput = extractKeyOutput(results[i].Message, 150) } // Default: summary mode (context-efficient) @@ -473,12 +476,14 @@ Usage: %[1]s resume "task" [workdir] %[1]s resume - [workdir] %[1]s --parallel Run tasks in parallel (config from stdin) + %[1]s --parallel --full-output Run tasks in parallel with full output (legacy) %[1]s --version %[1]s --help Parallel mode examples: %[1]s --parallel < tasks.txt echo '...' | %[1]s --parallel + %[1]s --parallel --full-output < tasks.txt %[1]s --parallel <<'EOF' Environment Variables: diff --git a/codeagent-wrapper/main_test.go b/codeagent-wrapper/main_test.go index ce42caf..b4ac34a 100644 --- a/codeagent-wrapper/main_test.go +++ b/codeagent-wrapper/main_test.go @@ -2972,6 +2972,46 @@ test` } } +func TestRunParallelWithFullOutput(t *testing.T) { + defer resetTestHooks() + cleanupLogsFn = func() (CleanupStats, error) { return CleanupStats{}, nil } + + oldArgs := os.Args + t.Cleanup(func() { os.Args = oldArgs }) + os.Args = []string{"codeagent-wrapper", "--parallel", "--full-output"} + + stdinReader = strings.NewReader(`---TASK--- +id: T1 +---CONTENT--- +noop`) + t.Cleanup(func() { stdinReader = os.Stdin }) + + orig := runCodexTaskFn + runCodexTaskFn = func(task TaskSpec, timeout int) TaskResult { + return TaskResult{TaskID: task.ID, ExitCode: 0, Message: "full output marker"} + } + t.Cleanup(func() { runCodexTaskFn = orig }) + + out := captureOutput(t, func() { + if code := run(); code != 0 { + t.Fatalf("run exit = %d, want 0", code) + } + }) + + if !strings.Contains(out, "=== Parallel Execution Summary ===") { + t.Fatalf("output missing full-output header, got %q", out) + } + if !strings.Contains(out, "--- Task: T1 ---") { + t.Fatalf("output missing task block, got %q", out) + } + if !strings.Contains(out, "full output marker") { + t.Fatalf("output missing task message, got %q", out) + } + if strings.Contains(out, "=== Execution Report ===") { + t.Fatalf("output should not include summary-only header, got %q", out) + } +} + func TestParallelInvalidBackend(t *testing.T) { defer resetTestHooks() cleanupLogsFn = func() (CleanupStats, error) { return CleanupStats{}, nil } diff --git a/codeagent-wrapper/utils.go b/codeagent-wrapper/utils.go index 464d574..877f019 100644 --- a/codeagent-wrapper/utils.go +++ b/codeagent-wrapper/utils.go @@ -75,9 +75,9 @@ func getEnv(key, defaultValue string) string { } type logWriter struct { - prefix string - maxLen int - buf bytes.Buffer + prefix string + maxLen int + buf bytes.Buffer dropped bool } @@ -205,6 +205,55 @@ func truncate(s string, maxLen int) string { return s[:maxLen] + "..." } +// safeTruncate safely truncates string to maxLen, avoiding panic and UTF-8 corruption. +func safeTruncate(s string, maxLen int) string { + if maxLen <= 0 || s == "" { + return "" + } + + runes := []rune(s) + if len(runes) <= maxLen { + return s + } + + if maxLen < 4 { + return string(runes[:1]) + } + + cutoff := maxLen - 3 + if cutoff <= 0 { + return string(runes[:1]) + } + if len(runes) <= cutoff { + return s + } + return string(runes[:cutoff]) + "..." +} + +// sanitizeOutput removes ANSI escape sequences and control characters. +func sanitizeOutput(s string) string { + var result strings.Builder + inEscape := false + for i := 0; i < len(s); i++ { + if s[i] == '\x1b' && i+1 < len(s) && s[i+1] == '[' { + inEscape = true + i++ // skip '[' + continue + } + if inEscape { + if (s[i] >= 'A' && s[i] <= 'Z') || (s[i] >= 'a' && s[i] <= 'z') { + inEscape = false + } + continue + } + // Keep printable chars and common whitespace. + if s[i] >= 32 || s[i] == '\n' || s[i] == '\t' { + result.WriteByte(s[i]) + } + } + return result.String() +} + func min(a, b int) int { if a < b { return a @@ -240,19 +289,12 @@ func extractMessageSummary(message string, maxLen int) string { continue } // Found a meaningful line - if len(line) <= maxLen { - return line - } - // Truncate long line - return line[:maxLen-3] + "..." + return safeTruncate(line, maxLen) } // Fallback: truncate entire message clean := strings.TrimSpace(message) - if len(clean) <= maxLen { - return clean - } - return clean[:maxLen-3] + "..." + return safeTruncate(clean, maxLen) } // extractCoverage extracts coverage percentage from task output @@ -262,20 +304,36 @@ func extractCoverage(message string) string { return "" } - // Common coverage patterns - patterns := []string{ - // pytest: "TOTAL ... 92%" - // jest: "All files ... 92%" - // go: "coverage: 92.0% of statements" + trimmed := strings.TrimSpace(message) + if strings.HasSuffix(trimmed, "%") && !strings.Contains(trimmed, "\n") { + if num, err := strconv.ParseFloat(strings.TrimSuffix(trimmed, "%"), 64); err == nil && num >= 0 && num <= 100 { + return trimmed + } } - _ = patterns // placeholder for future regex if needed + + coverageKeywords := []string{"file", "stmt", "branch", "line", "coverage", "total"} lines := strings.Split(message, "\n") for _, line := range lines { lower := strings.ToLower(line) - // Look for coverage-related lines - if !strings.Contains(lower, "coverage") && !strings.Contains(lower, "total") { + hasKeyword := false + tokens := strings.FieldsFunc(lower, func(r rune) bool { return r < 'a' || r > 'z' }) + for _, token := range tokens { + for _, kw := range coverageKeywords { + if strings.HasPrefix(token, kw) { + hasKeyword = true + break + } + } + if hasKeyword { + break + } + } + if !hasKeyword { + continue + } + if !strings.Contains(line, "%") { continue } @@ -323,40 +381,40 @@ func extractFilesChanged(message string) []string { var files []string seen := make(map[string]bool) + exts := []string{".ts", ".tsx", ".js", ".jsx", ".go", ".py", ".rs", ".java", ".vue", ".css", ".scss", ".md", ".json", ".yaml", ".yml", ".toml"} lines := strings.Split(message, "\n") for _, line := range lines { line = strings.TrimSpace(line) // Pattern 1: "Modified: path/to/file.ts" or "Created: path/to/file.ts" + matchedPrefix := false for _, prefix := range []string{"Modified:", "Created:", "Updated:", "Edited:", "Wrote:", "Changed:"} { if strings.HasPrefix(line, prefix) { file := strings.TrimSpace(strings.TrimPrefix(line, prefix)) + file = strings.Trim(file, "`,\"'()[],:") + file = strings.TrimPrefix(file, "@") if file != "" && !seen[file] { files = append(files, file) seen[file] = true } + matchedPrefix = true + break } } + if matchedPrefix { + continue + } - // Pattern 2: Lines that look like file paths (contain / and end with common extensions) - if strings.Contains(line, "/") { - for _, ext := range []string{".ts", ".tsx", ".js", ".jsx", ".go", ".py", ".rs", ".java", ".vue", ".css", ".scss"} { - if strings.HasSuffix(line, ext) || strings.Contains(line, ext+" ") || strings.Contains(line, ext+",") { - // Extract the file path - parts := strings.Fields(line) - for _, part := range parts { - part = strings.Trim(part, "`,\"'()[]") - if strings.Contains(part, "/") && !seen[part] { - for _, e := range []string{".ts", ".tsx", ".js", ".jsx", ".go", ".py", ".rs", ".java", ".vue", ".css", ".scss"} { - if strings.HasSuffix(part, e) { - files = append(files, part) - seen[part] = true - break - } - } - } - } + // Pattern 2: Tokens that look like file paths (allow root files, strip @ prefix). + parts := strings.Fields(line) + for _, part := range parts { + part = strings.Trim(part, "`,\"'()[],:") + part = strings.TrimPrefix(part, "@") + for _, ext := range exts { + if strings.HasSuffix(part, ext) && !seen[part] { + files = append(files, part) + seen[part] = true break } } @@ -408,8 +466,18 @@ func extractTestResults(message string) (passed, failed int) { } } + // go test style: "ok ... 12 tests" + if passed == 0 { + if idx := strings.Index(line, "test"); idx != -1 { + num := extractNumberBefore(line, idx) + if num > 0 { + passed = num + } + } + } + // If we found both, stop - if passed > 0 || failed > 0 { + if passed > 0 && failed > 0 { break } } @@ -472,10 +540,7 @@ func extractKeyOutput(message string, maxLen int) string { } content = strings.TrimSpace(content) if len(content) > 0 { - if len(content) <= maxLen { - return content - } - return content[:maxLen-3] + "..." + return safeTruncate(content, maxLen) } } } @@ -491,18 +556,12 @@ func extractKeyOutput(message string, maxLen int) string { if len(line) < 20 { continue } - if len(line) <= maxLen { - return line - } - return line[:maxLen-3] + "..." + return safeTruncate(line, maxLen) } // Fallback: truncate entire message clean := strings.TrimSpace(message) - if len(clean) <= maxLen { - return clean - } - return clean[:maxLen-3] + "..." + return safeTruncate(clean, maxLen) } // extractCoverageGap extracts what's missing from coverage reports @@ -615,8 +674,5 @@ func extractErrorDetail(message string, maxLen int) string { // Join and truncate result := strings.Join(errorLines, " | ") - if len(result) > maxLen { - return result[:maxLen-3] + "..." - } - return result + return safeTruncate(result, maxLen) } diff --git a/codeagent-wrapper/utils_test.go b/codeagent-wrapper/utils_test.go new file mode 100644 index 0000000..98a7427 --- /dev/null +++ b/codeagent-wrapper/utils_test.go @@ -0,0 +1,143 @@ +package main + +import ( + "fmt" + "reflect" + "strings" + "testing" +) + +func TestExtractCoverage(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + {"bare int", "92%", "92%"}, + {"bare float", "92.5%", "92.5%"}, + {"coverage prefix", "coverage: 92%", "92%"}, + {"total prefix", "TOTAL 92%", "92%"}, + {"all files", "All files 92%", "92%"}, + {"empty", "", ""}, + {"no number", "coverage: N/A", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := extractCoverage(tt.in); got != tt.want { + t.Fatalf("extractCoverage(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} + +func TestExtractTestResults(t *testing.T) { + tests := []struct { + name string + in string + wantPassed int + wantFailed int + }{ + {"pytest one line", "12 passed, 2 failed", 12, 2}, + {"pytest split lines", "12 passed\n2 failed", 12, 2}, + {"jest format", "Tests: 2 failed, 12 passed, 14 total", 12, 2}, + {"go test style count", "ok\texample.com/foo\t0.12s\t12 tests", 12, 0}, + {"zero counts", "0 passed, 0 failed", 0, 0}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + passed, failed := extractTestResults(tt.in) + if passed != tt.wantPassed || failed != tt.wantFailed { + t.Fatalf("extractTestResults(%q) = (%d, %d), want (%d, %d)", tt.in, passed, failed, tt.wantPassed, tt.wantFailed) + } + }) + } +} + +func TestExtractFilesChanged(t *testing.T) { + tests := []struct { + name string + in string + want []string + }{ + {"root file", "Modified: main.go\n", []string{"main.go"}}, + {"path file", "Created: codeagent-wrapper/utils.go\n", []string{"codeagent-wrapper/utils.go"}}, + {"at prefix", "Updated: @codeagent-wrapper/main.go\n", []string{"codeagent-wrapper/main.go"}}, + {"token scan", "Files: @main.go, @codeagent-wrapper/utils.go\n", []string{"main.go", "codeagent-wrapper/utils.go"}}, + {"space path", "Modified: dir/with space/file.go\n", []string{"dir/with space/file.go"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := extractFilesChanged(tt.in); !reflect.DeepEqual(got, tt.want) { + t.Fatalf("extractFilesChanged(%q) = %#v, want %#v", tt.in, got, tt.want) + } + }) + } + + t.Run("limits to first 10", func(t *testing.T) { + var b strings.Builder + for i := 0; i < 12; i++ { + fmt.Fprintf(&b, "Modified: file%d.go\n", i) + } + got := extractFilesChanged(b.String()) + if len(got) != 10 { + t.Fatalf("len(files)=%d, want 10: %#v", len(got), got) + } + for i := 0; i < 10; i++ { + want := fmt.Sprintf("file%d.go", i) + if got[i] != want { + t.Fatalf("files[%d]=%q, want %q", i, got[i], want) + } + } + }) +} + +func TestSafeTruncate(t *testing.T) { + tests := []struct { + name string + in string + maxLen int + want string + }{ + {"empty", "", 4, ""}, + {"zero maxLen", "hello", 0, ""}, + {"one rune", "你好", 1, "你"}, + {"two runes no truncate", "你好", 2, "你好"}, + {"three runes no truncate", "你好", 3, "你好"}, + {"two runes truncates long", "你好世界", 2, "你"}, + {"three runes truncates long", "你好世界", 3, "你"}, + {"four with ellipsis", "你好世界啊", 4, "你..."}, + {"emoji", "🙂🙂🙂🙂🙂", 4, "🙂..."}, + {"no truncate", "你好世界", 4, "你好世界"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := safeTruncate(tt.in, tt.maxLen); got != tt.want { + t.Fatalf("safeTruncate(%q, %d) = %q, want %q", tt.in, tt.maxLen, got, tt.want) + } + }) + } +} + +func TestSanitizeOutput(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + {"ansi", "\x1b[31mred\x1b[0m", "red"}, + {"control chars", "a\x07b\r\nc\t", "ab\nc\t"}, + {"normal", "hello\nworld\t!", "hello\nworld\t!"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := sanitizeOutput(tt.in); got != tt.want { + t.Fatalf("sanitizeOutput(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} diff --git a/development-essentials/.DS_Store b/development-essentials/.DS_Store deleted file mode 100644 index a8b3149ff92aa1383d9586c61346a6ad427896a2..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6148 zcmeHKu};G<5IvU&1%XgUMh}P$r0@l46&6OiGSD_CASF#D=)@BKflpxIGuZehHs1NH zG-+BY7KG4UWIyL}XFoqjaZE&JxJVv7nT0>6GS_ zccRtt7Zu>OJESw3V#UVPT)zcnbWgK9i$>G3m<##9Zi_F@oV(*^fB0; zlm0koe%Iv1?=S6Q1{vPJ89mZNoA=;k^?8%$+j-5i)kje-d2Nbm^KdeSP_v{0*BD&V zRW8?8;L?d{^7*{1`K*asa{4Z=&FdXQs;L61fGSWcfIXWn+!xeZ6;K6Kfm#9nK3EjS z*kdK=J{_ps5dav#?F?=AF9x*i0LC6GL3m(BQh}0c{1L-QI{enh#U3j`Nhjk*Mn8UJ z<4-8YjSjzc;bdY#tyKY4psm1;-1d3@zu0{KZzt)UDxeDdD+NrDjFKUyO sC^sl<99Id}DQLK@7_qz+AE7vd-*N*Od#nWEf$5KclR+z0;8zv+0#+S(EC2ui diff --git a/docs/CODEAGENT-WRAPPER.md b/docs/CODEAGENT-WRAPPER.md index dc7ae23..f8a589d 100644 --- a/docs/CODEAGENT-WRAPPER.md +++ b/docs/CODEAGENT-WRAPPER.md @@ -134,41 +134,39 @@ EOF ``` **Output Modes:** -- **Summary (default)**: Structured report with task results, verification, and review summary. +- **Summary (default)**: Structured report with extracted `Did/Files/Tests/Coverage`, plus a short action summary. - **Full (`--full-output`)**: Complete task messages included. Use only for debugging. **Summary Output Example:** ``` -=== Parallel Execution Summary === -Total: 3 | Success: 2 | Failed: 1 -Coverage Warning: 1 task(s) below target +=== Execution Report === +3 tasks | 2 passed | 1 failed | 1 below 90% ## Task Results -### backend_api ✓ -Changes: src/auth/login.ts, src/auth/middleware.ts -Output: "Implemented /api/login endpoint with JWT authentication" -Verify: 12 tests passed, coverage 92% (target: 90%) +### backend_api ✓ 92% +Did: Implemented /api/users CRUD endpoints +Files: backend/users.go, backend/router.go +Tests: 12 passed Log: /tmp/codeagent-xxx.log -### frontend_form ✓ -Changes: src/components/LoginForm.tsx -Output: "Created responsive login form with validation" -Verify: 8 tests passed, coverage 88% (target: 90%) ⚠️ BELOW TARGET +### frontend_form ⚠️ 88% (below 90%) +Did: Created login form with validation +Files: frontend/LoginForm.tsx +Tests: 8 passed +Gap: lines not covered: frontend/LoginForm.tsx:42-47 Log: /tmp/codeagent-yyy.log -### integration_tests ✗ +### integration_tests ✗ FAILED Exit code: 1 Error: Assertion failed at line 45 -Output: "Expected status 200 but got 401" +Detail: Expected status 200 but got 401 Log: /tmp/codeagent-zzz.log -## Summary for Review -- 2/3 tasks completed -- Issues requiring attention: - - integration_tests: Assertion failed at line 45 - - frontend_form: coverage 88% < 90% -- Action needed: fix 1 failed task(s), improve coverage for 1 task(s) +## Summary +- 2/3 completed successfully +- Fix: integration_tests (Assertion failed at line 45) +- Coverage: frontend_form ``` **Parallel Task Format:** diff --git a/requirements-driven-workflow/.DS_Store b/requirements-driven-workflow/.DS_Store deleted file mode 100644 index 09e97aabd60743a622481651685bdea1daf7a0da..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6148 zcmeHKK~BRk5L~wv3Id@XIpzZ(9XC zA#PeKE(oFB$R202YmdiK921clRcS;tB%%O?vA2(EgYk1N9qT#IA<)=9DoW{+PN<}; z6RnQFr~t3s5uMTuD>k9_`c*Wi1ue6AG@ccCDdhXBUGaI5B-5;zA})Aey+%*BZ)dx6 z(jVuPS8ZPW{?Z<1Fvt71q#L^Sc~>v%&)YnMyN0LNM_zpK+7{#Ucw`;{)GR2+H364& zolA7TDW=Wm`Jv&{>8s7@yEva$%)Hc80aZX1_*4LUHd}BYsI@Af3aA2&0{ndlQ5a)~ zwV?fUpt45*-~euGXxo1=V8{+&?64Mu2WBJ{D5=IBF^r_cAG)~MVJ#@>WbDZ3$Bk^< z3B}ma;SX&%nOIP3RX`Q+73js^YQZK24Z9U1mbc<#6l?H@+yKT7Ye9Hm`XgXv&`K5fRRul(smXaw diff --git a/skills/.DS_Store b/skills/.DS_Store deleted file mode 100644 index 01481f84fa1aca7358dcb3e48e59907a570d4c3d..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6148 zcmeHK%Sr<=6ur?Z^|9!ppma407lH^DT+3MM7xV}0W2l9WQ|k;Ux)>0bf(z@$o#-$4 z3vT;Ko|}YeVq0h{h)6CZCy#rRlhaAk5Rs@Ptuj%8i1J8`xpia{jQcr_*^+K40)-r- zPHhV4oZ@bK%-c0M1)KtZO#$A!Yt*EOTG+Y9{w>#AQ7oaeUW$NT*0_?3XH( lA;|P~EDL-UZzD-VpT`BDuQ64K7MS}ZAZ2iwQ{Yz>_yQC-