fix(agent): tick waiting progress during long provider calls (#256)

On a 90-file PR (+5848/-842) a single provider call legitimately runs for
many minutes (observed 4m18s for turn 1; turn 3 exceeded 8m). Progress was
only reported at turn boundaries, tool calls, and between retries — never
while a call was in flight — so the stalled-review watchdog (which cancels
after N minutes of progress silence) killed the review mid-call, discarding
12+ minutes of work to a full retry ("review stalled: no progress for 8m0s").

Add callWithWaitProgress: while a provider call is in flight, tick progress
with "waiting on <label> (elapsed)" every minute, resetting the watchdog.
Wired into retryProviderCall (anthropic/openai) and the codex post loop. A
genuinely dead call is still bounded by the overall review timeout, as any
sub-stall-timeout call already was. Tests run under -race (the ticker calls
progress from a goroutine; the watchdog reset channel is concurrency-safe).

Author: vanducng

internal/engine/agent/codex.go

@@ -331,7 +331,9 @@ func (a *codexAgent) post(ctx stdctx.Context, retry config.ProviderRetry, progre
 		if err := ctx.Err(); err != nil {
 			return nil, err
 		}
-		resp, err := a.postOnce(ctx, body)
+		resp, err := callWithWaitProgress(ctx, progress, "codex.responses", func() (*codexResp, error) {
+			return a.postOnce(ctx, body)
+		})
 		if err == nil {
 			return resp, nil
 		}

internal/engine/agent/provider_retry.go

@@ -34,6 +34,41 @@ func (r providerRetryReason) String() string {
 	return r.code
 }
 
+// providerWaitProgressInterval is how often an in-flight provider call reports
+// waiting progress. A single call on a huge diff can legitimately run for many
+// minutes (observed: 4-8+ min per turn on a 90-file PR); without these ticks the
+// stalled-review watchdog (which cancels after N minutes of progress silence)
+// kills a review the provider is still working on. Var so tests can shorten it.
+var providerWaitProgressInterval = time.Minute
+
+// callWithWaitProgress runs call, ticking progress with "waiting on <label>
+// (elapsed)" every providerWaitProgressInterval until it returns, so a long
+// provider call keeps resetting the stalled-review watchdog. A genuinely dead
+// call is still bounded by the overall review timeout.
+func callWithWaitProgress[T any](ctx stdctx.Context, progress func(string), label string, call func() (T, error)) (T, error) {
+	if progress == nil {
+		return call()
+	}
+	done := make(chan struct{})
+	defer close(done)
+	start := time.Now()
+	go func() {
+		ticker := time.NewTicker(providerWaitProgressInterval)
+		defer ticker.Stop()
+		for {
+			select {
+			case <-done:
+				return
+			case <-ctx.Done():
+				return
+			case <-ticker.C:
+				progress(fmt.Sprintf("waiting on %s (%s)", label, time.Since(start).Round(time.Second)))
+			}
+		}
+	}()
+	return call()
+}
+
 func retryProviderCall[T any](ctx stdctx.Context, retry config.ProviderRetry, progress func(string), label string, call func() (T, error), classify func(error) error, retryable func(error) (providerRetryReason, bool)) (T, error) {
 	policy := resolveProviderRetryPolicy(retry, config.DefaultProviderRetryMaxRetries)
 	var zero T
@@ -42,7 +77,7 @@ func retryProviderCall[T any](ctx stdctx.Context, retry config.ProviderRetry, pr
 		if err := ctx.Err(); err != nil {
 			return zero, err
 		}
-		out, err := call()
+		out, err := callWithWaitProgress(ctx, progress, label, call)
 		if err == nil {
 			return out, nil
 		}

internal/engine/agent/provider_retry_test.go

@@ -0,0 +1,99 @@
+package agent
+
+import (
+	stdctx "context"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/vanducng/miu-cr/internal/config"
+)
+
+// progressRecorder collects progress messages; safe for the ticker goroutine.
+type progressRecorder struct {
+	mu   sync.Mutex
+	msgs []string
+}
+
+func (r *progressRecorder) record(s string) {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	r.msgs = append(r.msgs, s)
+}
+
+func (r *progressRecorder) waiting() []string {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	var out []string
+	for _, m := range r.msgs {
+		if strings.Contains(m, "waiting on") {
+			out = append(out, m)
+		}
+	}
+	return out
+}
+
+func shortWaitInterval(t *testing.T, d time.Duration) {
+	t.Helper()
+	old := providerWaitProgressInterval
+	providerWaitProgressInterval = d
+	t.Cleanup(func() { providerWaitProgressInterval = old })
+}
+
+// A provider call that outlives the tick interval must emit waiting progress —
+// without it, the stalled-review watchdog (progress-silence based) cancels a
+// review the provider is still working on (observed on a 90-file PR whose
+// single turn ran past the 8m stall timeout).
+func TestCallWithWaitProgressTicksDuringLongCall(t *testing.T) {
+	shortWaitInterval(t, 10*time.Millisecond)
+	rec := &progressRecorder{}
+
+	out, err := callWithWaitProgress(stdctx.Background(), rec.record, "anthropic.messages", func() (string, error) {
+		time.Sleep(80 * time.Millisecond)
+		return "ok", nil
+	})
+	if err != nil || out != "ok" {
+		t.Fatalf("call result = %q, %v", out, err)
+	}
+	ticks := rec.waiting()
+	if len(ticks) == 0 {
+		t.Fatal("expected waiting-progress ticks during a long provider call (stall watchdog would fire without them)")
+	}
+	if !strings.Contains(ticks[0], "waiting on anthropic.messages") {
+		t.Fatalf("tick must name the call: %q", ticks[0])
+	}
+}
+
+func TestCallWithWaitProgressNilProgress(t *testing.T) {
+	shortWaitInterval(t, time.Millisecond)
+	out, err := callWithWaitProgress(stdctx.Background(), nil, "x", func() (int, error) {
+		time.Sleep(10 * time.Millisecond)
+		return 7, nil
+	})
+	if err != nil || out != 7 {
+		t.Fatalf("call result = %d, %v", out, err)
+	}
+}
+
+// The wiring: retryProviderCall must tick waiting progress while its call is in
+// flight (this is the path the anthropic/openai review loops use).
+func TestRetryProviderCallTicksWaitingProgress(t *testing.T) {
+	shortWaitInterval(t, 10*time.Millisecond)
+	rec := &progressRecorder{}
+
+	out, err := retryProviderCall(stdctx.Background(), config.ProviderRetry{}, rec.record, "openai.chat",
+		func() (string, error) {
+			time.Sleep(60 * time.Millisecond)
+			return "done", nil
+		},
+		func(err error) error { return err },
+		func(err error) (providerRetryReason, bool) { return providerRetryReason{}, false },
+	)
+	if err != nil || out != "done" {
+		t.Fatalf("call result = %q, %v", out, err)
+	}
+	if len(rec.waiting()) == 0 {
+		t.Fatal("retryProviderCall must emit waiting-progress ticks during a long call")
+	}
+}