Unrecovered panic in a per-file review goroutine crashes the whole run (missing recover, CWE-248)

[Aravindargutus]

OpenCodeReview Version

V1.3.19

Operating System

macOS (Apple Silicon)

Installation Method

npm (global)

LLM Provider

Anthropic (Claude)

Bug Description

dispatchSubtasks reviews each changed file in its own goroutine and is built
around per-file fault isolation: an error from executeSubtask is caught,
counted, recorded as a subtask_error warning, and the other files continue
(internal/agent/agent.go:456-475). The same applies to CommentWorkerPool.Submit
(agent.go:206-216). But neither goroutine has a recover() — their deferred
calls only do wg.Done() and semaphore release.

In Go, an unrecovered panic in any goroutine terminates the entire process.
So a panic while reviewing one file (from any latent bug, an unexpected
LLM/SDK response shape, or a malformed-diff edge case) does not fail just that
file the way an error does — it aborts the whole ocr run and discards every
other file's results. This silently defeats the per-file isolation the code
otherwise implements.

Steps to Reproduce

The crash is in the goroutine contract itself: dispatchSubtasks spawns one
goroutine per file whose only deferred work is wg.Done() + semaphore release,
with no recover(). Any panic in one of those goroutines aborts the whole
process (Go terminates on an unrecovered panic in any goroutine), discarding all
other files' results — whereas an error from the same file would be isolated
and counted.

A. Minimal, self-contained reproduction of the exact pattern

Save and run this (it mirrors agent.go:456-475 — per-file goroutine, defer
wg.Done()+sem release, no recover):

package main

import ("fmt"; "sync"; "sync/atomic")

func main() {
    files := []string{"a.go", "b.go", "EVIL.go", "c.go", "d.go"}
    var wg sync.WaitGroup
    sem := make(chan struct{}, 8)
    var completed int64
    for _, f := range files {
        wg.Add(1); sem <- struct{}{}
        go func(name string) {
            defer wg.Done()
            defer func() { <-sem }() // current code: NO recover
            executeSubtask(name)
            atomic.AddInt64(&completed, 1)
        }(f)
    }
    wg.Wait()
    fmt.Printf("review finished; %d/%d files completed\n", completed, len(files))
}
func executeSubtask(name string) {
    if name == "EVIL.go" { panic("index out of range while reviewing " + name) }
}

### Expected Behavior

A failure isolated to one file's review — including a panic — should be caught,
recorded as a per-file failure/warning, and leave the other concurrent reviews
unaffected, exactly as `error` returns already are.


### Logs / Error Output

```shell

Additional Context

Actual behaviour

A panic in any subtask (or comment-pool) goroutine propagates uncaught and
crashes the entire process, losing all in-flight and completed-but-uncollected
results.

Suggested fix

Add a recover() to each worker goroutine that converts a panic into the same
recorded per-file failure path used for errors.