[code-review] purgeTerminalAgents removes agents whose goroutine is still alive — done channel never closes

[topcheer]

purgeTerminalAgents removes agents whose goroutine is still alive — done channel never closes

Severity: Low-Medium

File and Lines

internal/subagent/manager.go, lines 1110-1141 (purge), 783-790 (Complete early-return), 658-662 (Cancel)

Problem Description

purgeTerminalAgents checks only Status (Completed/Failed/Cancelled) to decide if an agent can be safely removed. But Cancel() sets Status to Cancelled without closing the done channel — it relies on the goroutine's Complete() call to do that. If the agent is purged from the map before its goroutine calls Complete(), the Complete() lookup fails (ok=false → early return), closeDone() is never called, and done stays open forever.

Trigger Scenario

1. Agent A is Cancel'd (Status=Cancelled, EndedAt=T1) but goroutine still running (slow LLM shutdown)
2. 20+ other agents complete with EndedAt > T1
3. Watchdog ticker fires purgeTerminalAgents — A is oldest terminal → deleted
4. A's goroutine returns, calls Complete("A") → m.agents["A"] not found → early return
5. closeDone() never called → done channel permanently open

Why it's real

• purgeTerminalAgents does not check whether the goroutine has finished (goroutineStarted or done closed)
• The goroutineStarted field exists (line 102) but is not used by purge
• Cancel() explicitly does NOT close done (line 658 comment)
• Complete() early-returns if agent not in map (line 786-789)

Mitigation

• Requires 20+ terminal agents simultaneously (rare)
• CancelAll has timeout fallback (won't permanently deadlock)
• CancelAll only collects Running/Pending agents in doneChs, so already-cancelled agents aren't waited on

Fix Suggestion

In purgeTerminalAgents, skip agents whose done channel is not yet closed:

if sa.done != nil {
    select {
    case <-sa.done:
        // goroutine confirmed exited — safe to purge
    default:
        continue // goroutine still running — skip
    }
}

[topcheer]

已修复 (commit babf0f5)。

修复：purgeTerminalAgents 现在使用 select 检查 done channel 是否已关闭。未关闭（goroutine 仍在运行）的 agent 会被跳过，防止 Complete() 找不到 agent 导致 done channel 永久打开。