Review Summary

This PR adds Copilot tool call removal (removeCopilotReportTaskToolCall) and changes findGenericSlimMessageBox to use regex to support Copilot's 4-line message box pattern. All tests pass. Overall risk: low-medium.

Issues

P2 (consideration): Significant code duplication in lib/msgfmt/format_tool_call.go

removeCopilotReportTaskToolCall (L128-188) is a near-identical copy of removeClaudeReportTaskToolCall (L13-74). The diff between them is only ~10 lines of actual logic differences (start marker, end-check, loop bounds). Everything else — toolCallRange tracking, malformed handling, reverse removal loop, TrimLeft cleanup — is copy-pasted verbatim (~50 lines of scaffolding).

This is the third tool-call remover and the second time the Claude scaffolding has been duplicated. Adding a fourth agent will mean a third copy.

suggestion: Extract a shared helper parameterized on the agent-specific parts. Something like:

type toolCallMatcher struct {
    isStart func(line string) bool
    isEnd   func(line, prevLine, nextLine string) bool
}

func removeReportTaskToolCall(msg string, m toolCallMatcher) (string, []string) { ... }

Each agent would only supply the marker detection closures. This would eliminate ~50 lines of duplication per agent and make the malformed handling consistent by construction.

Not blocking since the current code works correctly, but this will compound quickly.

P3 (nitpick): Search direction change in lib/msgfmt/message_box.go

The old findGenericSlimMessageBox iterated backward from len(lines)-3 (finding the last match in the window). The new regex FindStringIndex finds the first match in the window. This is a semantic difference.

I confirmed the regex correctly handles both the 3-line box and the new 4-line Copilot box, and the old code fails on the 4-line pattern (returns -1). The regex change is well-motivated.

In practice, there is only one message box in the search window, so the direction change is unlikely to matter. But it's worth noting as an undocumented behavioral change. A // Note: finds the first (topmost) match in the window comment would help future readers.

P3 (thought): Inconsistent pattern with other message box finders

Every other box finder in the file uses strings.Contains line iteration:

• findGreaterThanMessageBox
• removeCodexMessageBox
• removeOpencodeMessageBox
• removeAmpMessageBox

The regex is the odd one out. That said, it is genuinely cleaner for the 3-or-4-line variadic pattern, and the compiled regexp.MustCompile at package level is efficient. I think this is the right call — just noting the inconsistency.

P4 (thought): Speculative malformed handling

Lines 159-160 note "This case has not yet been observed in Copilot" but include the full malformed handling code path. This is fine as defensive coding in a streaming context, but it contributes to the duplication burden. If the shared helper from the P2 suggestion existed, this would come for free.

Questions

1. Was the search direction change in findGenericSlimMessageBox intentional, or a side effect of switching to regex?
2. Is there appetite to extract the shared tool-call removal scaffolding now, or defer to a follow-up?

Validation

• ✅ All existing tests pass on main
• ✅ All tests pass on the PR branch (including new copilot/remove-task-tool-call)
• ✅ Confirmed the old findGenericSlimMessageBox returns -1 for the 4-line Copilot box; the new regex correctly matches it
• ✅ Confirmed the regex still correctly matches the existing 3-line box pattern
• ✅ Tested a false-positive edge case (table borders with | near a real message box) — both old and new code return the same correct result

🤖 This response was generated by Coder Agents.