fix(pi): keep turn open while Pi auto-retries transient errors (willRetry)

[Duliy]

Problem

Pi (earendil-works/pi-coding-agent) has a built-in agent-level auto-retry for transient provider failures (HTTP 429 rate limits, 5xx, overloaded, network errors). When a run ends with a retryable error, Pi emits agent_end with willRetry: true, then re-runs the agent loop and emits a fresh agent_start / agent_end cycle. (See dist/core/agent-session.js: _emit({ ...event, willRetry: this._willRetryAfterAgentEnd(event) }).)

cc-connect's pi adapter doesn't read willRetry, and surfaces every assistant errorMessage immediately as EventError (handleMessageEnd). The engine treats EventError as turn-fatal (core/engine.go: finalize failed card, push error to platform, return). As a result, on every transient 429:

1. The user immediately sees a hard error on the platform (e.g. Feishu);
2. The turn is closed from cc-connect's perspective (json mode: EventError ends the loop; rpc mode: first agent_end also emits EventResult Done);
3. Pi keeps retrying in the background and usually recovers within seconds, but the recovered run's events are drained as stale (drained stale events from previous turn) and the user never sees the result.

Real-world impact: with rate-limited providers (e.g. Kimi rate_limit_error: The engine is currently overloaded), users see "429 error → silence" on virtually every busy turn, even though Pi's retry succeeds almost every time. May be related to #1499 / #1498.

Fix

In agent/pi/session.go (shared by json and rpc modes):

• Buffer, don't emit: the latest assistant errorMessage goes into s.pendingErr instead of an immediate EventError. A subsequent healthy assistant message_end clears the buffer.
• willRetry keeps the turn open: on agent_end with willRetry: true, emit nothing — the retried run produces a fresh event cycle.
• Terminal agent_end flushes: with willRetry absent/false, a buffered error is emitted as EventError before the turn closes — the failure UX for truly-final errors is unchanged (and older Pi builds without willRetry behave exactly as before, just deferred by one event).
• sendJSON exit fallback: flushes pendingErr on process exit in case no terminal agent_end was observed.

Verified against pi-coding-agent 0.81: both --mode json and --mode rpc stream the same session events, so agent_end.willRetry is available in both modes.

Tests

• Updated TestHandleMessageEnd_AssistantError for deferred buffering.
• New: TestHandleEvent_AgentEndWillRetryKeepsTurnOpen, TestHandleEvent_AgentEndFlushesPendingError, TestHandleEvent_AgentEndRetrySuccessDropsPendingError.
• go build ./..., go vet ./agent/pi/, go test ./agent/pi/ and go test ./core/ all pass (go 1.25.0).

Notes

• No config or behavior change for other agents; the change is scoped to the pi adapter.
• A nice follow-up (out of scope): render a transient "retrying…" hint on the platform when willRetry is seen, e.g. via the progress card.

[luw2007]

+1. I reproduced the same Pi transient termination behavior through cc-connect: the model can emit terminated before any visible output, then recover on a subsequent attempt. This PR handles the authoritative Pi agent_end.willRetry signal across both JSON and RPC modes, which is preferable to adapter-local retry heuristics.

[Duliy]

Thanks for reproducing and confirming the issue! Your validation really helps. I agree — handling willRetry at the protocol level is much cleaner than each adapter implementing its own retry heuristics. Appreciate the +1 🙌

[chenhg5]

Conclusion: Approve

Overall assessment:

• This is exactly the kind of adapter-scoped fix we want for an upstream quirk: Pi's agent_end.willRetry=true was being ignored, so every transient provider failure (429 / 5xx / overloaded) was being surfaced as a turn-fatal EventError even though the agent was about to recover it within seconds. The PR fixes that by deferring the assistant errorMessage into a pendingErr buffer that is flushed only when the turn truly ends. The change is contained to agent/pi/session.go + its test file, does not touch the engine or any other adapter, and the buffer is flushed from one of two well-defined exit points (agent_end.willRetry=false and sendJSON exit). Three new tests cover the three observable paths (willRetry keeps the turn open, terminal flushes the error, successful retry drops the buffer), and TestHandleMessageEnd_AssistantError is correctly updated to assert deferred behavior. Tests are green.

Review scope:

• Reviewed agent/pi/session.go (the new pendingErr field, the handleEvent("agent_end") branch, the handleMessageEnd("assistant") change, and the sendJSON exit flush) plus agent/pi/pi_test.go.
• Focused on adapter scope, race safety (single-goroutine invariant), error UX on the truly-fatal path, and backward compatibility for Pi builds that don't emit willRetry.

✅ What looks good:

• Adapter-scoped, no engine or platform touches. This is the right shape for a quirk in one upstream agent's protocol — the engine continues to treat EventError as turn-fatal, and only the pi adapter decides when to emit it.
• Thread-safety argument is sound. pendingErr is documented as "Only written from handleEvent, which runs on a single goroutine per mode." Verified: in JSON mode, sendJSON is the calling goroutine and handleEvent runs in its own stdout-read loop synchronously inside sendJSON (no separate goroutine); in RPC mode, sendRPC only writes to stdin, while readLoopRPC is the only goroutine that calls handleEvent. So the "one writer per mode" invariant holds, no atomic / mutex needed.
• Truly-fatal errors still get reported. When agent_end.willRetry is false/absent, the buffered pendingErr is emitted before EventResult — so an actual final 400 / 500 still surfaces as EventError, just one event later. Verified by TestHandleEvent_AgentEndFlushesPendingError. Older Pi builds without willRetry behave exactly as before but with this one-event deferral.
• Robust exit-time flush. sendJSON flushes pendingErr on process exit in case no terminal agent_end was observed (e.g. agent crashed mid-retry). This closes the "Pi retries -> pi process crashes before the second agent_end" gap and matches the spirit of the stderr-based EventError already emitted in readLoopJSON.
• PR description is exemplary. "Problem / root cause / fix / verification / tests / notes" with explicit pointers to Pi's dist/core/agent-session.js for the willRetry semantics. Note that willRetry is documented as a boolean in the SDUI; the test uses both true and false literals and verifies they are handled correctly.

🚨/🔴 Must fix:

• None.

🟠 Should improve:

• Minor: "successful retry drops pending error" is asserted on, but the buffer-drop is only implicit. TestHandleEvent_AgentEndRetrySuccessDropsPendingError checks the event sequence but does not assert on s.pendingErr == "" after the second message_end. Adding that assertion would lock in the "healthy assistant message supersedes any earlier error" contract in handleMessageEnd. Quick one-liner.
• Minor: consider documenting the deferral in core.EventError doc comments / CHANGELOG. Other consumers (messaging platforms that key off EventError to surface a "failed" UX) may notice the one-event delay for older Pi builds. Worth noting in the adapter's own section of CHANGELOG so power users running a pinned Pi version understand the behavior change.

🔵 Optional:

• The PR description mentions a follow-up idea: render a transient "retrying…" hint on the platform when willRetry is seen (e.g. via the progress card). Consider filing that as a follow-up ticket; it is genuinely useful UX for users hitting rate-limited providers.
• A small comment near the new pendingErr field noting that older Pi builds (no willRetry) still rely on the terminal-agent_end flush path would help future readers grep for the protocol version constraint.

❓ Questions:

• Is there a CHANGELOG entry we should add for the one-event deferral of truly-final errors on older Pi builds? Probably yes — worth confirming with the author / PM.

Testing / Risk:

• Verified: go test -race ./agent/pi/... -count=1 -timeout 60s (7.0s, all green); new + updated tests pass. Full TestHandleEvent_* and TestHandleMessageEnd_* suite green.
• Blast radius is agent/pi/ only. No core/, no platform, no CLI touched.
• Backward compatibility:
  • Newer Pi (with willRetry): transient errors no longer surface mid-turn. Truly-final errors still surface, just one event later. This is the intended behavior change and unblocks users on rate-limited providers (Kimi, etc.).
  • Older Pi (without willRetry): truly-final errors surface one event later, with no other behavior change. See 🟠 / ❓ above.

Next step:

• Merge. Suggest adding the two 🟠 minor test/CHANGELOG items in a follow-up PR (or in this one if the author prefers). Solid first-time contribution from Duliy.

[Duliy]

Thanks for the thorough review! All items addressed in the rebased branch (b955b1d):

🟠 Done:

• TestHandleEvent_AgentEndRetrySuccessDropsPendingError now asserts s.pendingErr == "" after the healthy message_end, locking in the "healthy supersedes earlier error" contract in handleMessageEnd.
• pendingErr field comment now documents the older-Pi compatibility path (no willRetry → flushed at terminal agent_end / process exit, one event later).
• CHANGELOG: added an Unreleased / Fixed entry describing the fix and explicitly noting the one-event deferral for older Pi builds (answering your ❓ — yes, entry added).

🔵 Follow-up: I'll file the transient "retrying…" platform hint as a separate ticket so it doesn't block this one.

Rebased onto latest main (6c86079). Re-verified: go build / go vet / go test -race ./agent/pi/ / go test ./core/ all green (no_web tag, go 1.25.0).

From my side this is ready to merge — let me know if anything else is needed. 🙏