[code-review] a2a handler continueTask cancel race — resumed task context silently canceled by prior execute cleanup

[topcheer]

Problem

In internal/a2a/handler.go, continueTask (line 236-237) creates a new cancel context and directly overwrites h.cancels[taskID] without first calling the old cancel function. Meanwhile, the original execute() goroutine continues running after RequestInput and always reaches cleanupCancel(t.ID) at line 301.

This creates a race condition: if the original execute() goroutine reaches cleanupCancel after continueTask has stored the new cancel, it cancels the new context, causing the resumed task to fail or behave unexpectedly.

File and Lines

• internal/a2a/handler.go lines 235-237 (continueTask overwrites cancel without calling old)
• internal/a2a/handler.go line 301 (execute calls cleanupCancel unconditionally)

Trigger Scenario

Time  G1 (original execute)           G2 (continueTask)         cancels[taskID]
----  ------------------------------  ------------------------  ---------------
T1    RequestInput called                                        [oldCancel]
      (task → InputRequired)
T2    ...execution wrapping up...
T3                                     h.cancels[taskID] = cancel [newCancel]
T4    cleanupCancel(t.ID) called
      - c = cancels[taskID] = newCancel
      - c() — CANCELS THE NEW CONTEXT!
      - delete(cancels[taskID])                                []
T5                                     safego.Go(execute) starts
                                      with already-canceled context

1. Task reaches input-required state via RequestInput()
2. Original execute() goroutine continues past the work phase to line 301
3. Client sends follow-up message → continueTask() creates new cancel at line 236-237
4. Original execute() reaches cleanupCancel(t.ID) at line 301
5. cleanupCancel reads the map, finds the new cancel, calls it, and deletes it
6. The resumed task starts with an already-canceled context

Expected vs Actual Behavior

Expected: When resuming a task via continueTask, the new context should remain valid until the task completes or is explicitly canceled.

Actual: The old execute() goroutine's cleanupCancel races with continueTask and can cancel the new context, causing the resumed task to immediately fail or exhibit timeout/cancellation errors.

Severity

High — The resumed task silently receives a canceled context. This is difficult to reproduce and debug because it depends on precise timing between the original execution winding down and the client sending a follow-up.

Fix Suggestion

In continueTask, cancel the old context before creating the new one:

// Cancel the old context first (original execute may still be running)
if oldCancel, ok := h.cancels[taskID]; ok {
    oldCancel()
}
taskCtx, cancel := context.WithTimeout(context.Background(), h.timeout)
h.cancels[taskID] = cancel

Alternatively, cleanupCancel should only cancel if the task is in a terminal state, not input-required.

[topcheer]

已修复 — commit 72ad1e8

问题根因：continueTask 直接覆写 h.cancels[taskID]，但原 execute() goroutine 的 cleanupCancel 会读取新 cancel 并调用它，导致恢复的任务使用已被取消的 context。

修复内容：在 continueTask 创建新 context 之前，先调用旧 cancel 并从 map 中删除，防止 cleanupCancel 误删新 context。

验证：go build + go test -tags goolm 通过。