fix(pi): finalize turns on the run terminal so a reused process does not replay the previous answer #83

Merged

weiwen merged 1 commit from fm/evie-followup-repeat-diagnosis into main

2026-07-30 21:01:48 +08:00

weiwen commented

2026-07-30 20:57:30 +08:00

Owner

Problem

A follow-up turn made Evie replay the previous turn's answer, instantly (microseconds, no LLM call) instead of answering the new message. Reproduced live on matsutake (build mm47w0w…, pi 0.80.10, provider=opencode): a follow-up returned the prior answer in ~0.4ms, and its Pi tool call debug lines (emitted only by last_assistant_text, reachable only from the agent_end branch) proved it had parsed a leftover agent_end.

Regression from #81.

Mechanism

#81 added a turn_end{stop} return path to read_agent_response. In pi 0.80.10's stream a turn_end{stop} is followed by the run's own terminal events — agent_end, then agent_settled. Returning at turn_end{stop} handed control back while agent_end was still unread in the process's stdout.

The pi process is reused across turns, and PiProcess::send_message does reader.into_inner() between turns, which discards only the userspace BufReader buffer — not the kernel pipe. So the trailing agent_end (which restates the just-finished turn's assistant text) survived into the next turn, whose fresh reader consumed it first and returned it via last_assistant_text. The pre-#81 parser returned on agent_end, so it consumed the run fully and never stranded a text-bearing event — hence no repeat.

Reproduced end-to-end against the real pi 0.80.10 binary: the #81 parser delivered a stale answer on every follow-up; the pre-#81 parser did not.

Fix

Align the turn boundary with pi's run boundary so nothing is stranded:

A terminal turn_end now only records its text into TurnState (last terminal wins, so an auto_retry re-run overwrites the earlier attempt); it no longer finalizes the turn.
The turn is finalized at the run's true terminal: agent_end in the native schema (delivers directly, consuming itself so only the benign text-less agent_settled trails) or agent_settled in the turn schema (delivers the recorded response).
agent_settled still surfaces a recorded provider error and still skips a stray leading marker (so a run that produced neither doesn't finalize empty).
EOF fallback returns a recorded response for a provider that closes stdout without a final agent_settled.

This also removes a latent early-return on the first agent_end of an auto_retry run. #81's turn-schema/error handling is kept intact; this does not revert to returning on agent_end only (which would reintroduce #81's "turns never finalize").

Note on approach

The diagnosis proposed making agent_settled the sole terminal (with agent_end recording). That stalls two existing fixtures that emit agent_end on an open stream with no agent_settled (test_abort_after_agent_end_returns_response, fake_multi_responder) — they would hang to the idle timeout, and the drain-to-settle fallback hits the same wall. Keeping agent_end as the native terminal fixes the bug identically (it is the proven-correct pre-#81 native behavior) while touching no existing fixture and staying robust to an agent_end-without-agent_settled provider.

Regression test

reused_reader_does_not_replay_previous_turn drives read_agent_response twice over one continuous reader holding turn 1's full turn_end{stop} → agent_end → agent_settled tail, and asserts turn 2 delivers its own answer. This is the exact gap in #81's tests (they fed a single stream to one call and never ran a second turn over the leftover tail). Verified it fails on the pre-fix behavior (turn 2 returns "FIRST") and passes with the fix.

Validation

just check green: cargo fmt --check, cargo clippy -- -D warnings, cargo test (486 passed, incl. the new test and the existing pi/native + turn-schema tests).

## Problem A follow-up turn made Evie **replay the previous turn's answer**, instantly (microseconds, no LLM call) instead of answering the new message. Reproduced live on `matsutake` (build `mm47w0w…`, pi 0.80.10, `provider=opencode`): a follow-up returned the prior answer in ~0.4ms, and its `Pi tool call` debug lines (emitted only by `last_assistant_text`, reachable only from the `agent_end` branch) proved it had parsed a leftover `agent_end`. **Regression from #81.** ## Mechanism `#81` added a `turn_end{stop}` return path to `read_agent_response`. In pi 0.80.10's stream a `turn_end{stop}` is followed by the run's own terminal events — `agent_end`, then `agent_settled`. Returning at `turn_end{stop}` handed control back while `agent_end` was still unread in the process's stdout. The pi process is **reused across turns**, and `PiProcess::send_message` does `reader.into_inner()` between turns, which discards only the userspace `BufReader` buffer — **not** the kernel pipe. So the trailing `agent_end` (which restates the just-finished turn's assistant text) survived into the next turn, whose fresh reader consumed it first and returned it via `last_assistant_text`. The pre-#81 parser returned on `agent_end`, so it consumed the run fully and never stranded a text-bearing event — hence no repeat. Reproduced end-to-end against the real pi 0.80.10 binary: the #81 parser delivered a stale answer on every follow-up; the pre-#81 parser did not. ## Fix Align the turn boundary with pi's *run* boundary so nothing is stranded: - A terminal `turn_end` now only **records** its text into `TurnState` (last terminal wins, so an `auto_retry` re-run overwrites the earlier attempt); it no longer finalizes the turn. - The turn is finalized at the run's true terminal: **`agent_end`** in the native schema (delivers directly, consuming itself so only the benign text-less `agent_settled` trails) or **`agent_settled`** in the turn schema (delivers the recorded response). - `agent_settled` still surfaces a recorded provider error and still skips a stray leading marker (so a run that produced neither doesn't finalize empty). - EOF fallback returns a recorded response for a provider that closes stdout without a final `agent_settled`. This also removes a latent early-return on the first `agent_end` of an `auto_retry` run. #81's turn-schema/error handling is kept intact; this does **not** revert to returning on `agent_end` only (which would reintroduce #81's "turns never finalize"). ### Note on approach The diagnosis proposed making `agent_settled` the *sole* terminal (with `agent_end` recording). That stalls two existing fixtures that emit `agent_end` on an open stream with no `agent_settled` (`test_abort_after_agent_end_returns_response`, `fake_multi_responder`) — they would hang to the idle timeout, and the drain-to-settle fallback hits the same wall. Keeping `agent_end` as the native terminal fixes the bug identically (it is the proven-correct pre-#81 native behavior) while touching no existing fixture and staying robust to an `agent_end`-without-`agent_settled` provider. ## Regression test `reused_reader_does_not_replay_previous_turn` drives `read_agent_response` **twice over one continuous reader** holding turn 1's full `turn_end{stop}` → `agent_end` → `agent_settled` tail, and asserts turn 2 delivers its own answer. This is the exact gap in #81's tests (they fed a single stream to one call and never ran a second turn over the leftover tail). Verified it fails on the pre-fix behavior (turn 2 returns `"FIRST"`) and passes with the fix. ## Validation `just check` green: `cargo fmt --check`, `cargo clippy -- -D warnings`, `cargo test` (486 passed, incl. the new test and the existing pi/native + turn-schema tests).

weiwen added 1 commit

2026-07-30 20:57:30 +08:00

fix(pi): finalize turns on the run terminal so a reused process does not replay the previous answer

CI / check (pull_request) Successful in 1m42s

Details

PR Triage — label changes-requested reviews / triage-review (pull_request) Successful in 2s

Details

9203c6dbc5

A follow-up turn replayed the previous turn's answer (instantly, no LLM
call). Regression from #81.

#81 added a `turn_end{stop}` return path to `read_agent_response`. In pi
0.80.10's stream a `turn_end{stop}` is followed by the run's own terminal
events (`agent_end`, then `agent_settled`), so returning at `turn_end{stop}`
handed control back while `agent_end` was still unread in the process's
stdout. Because the pi process is reused across turns and `into_inner()`
discards only the userspace buffer (not the kernel pipe), that trailing
`agent_end` — which restates the just-finished turn's assistant text —
survived into the next turn, whose fresh reader consumed it first and
returned it via `last_assistant_text`. Confirmed live and reproduced against
the real pi 0.80.10 binary; the pre-#81 parser (which returned on `agent_end`)
did not exhibit it.

Fix: a terminal `turn_end` now only records its text into `TurnState`; the
turn is finalized at the run's true terminal instead — `agent_end` in the
native schema (which delivers directly, consuming itself) or `agent_settled`
in the turn schema (which delivers the recorded response). `agent_settled`
still surfaces a recorded error and still skips a stray leading marker, so a
run that produced neither does not finalize empty. An EOF fallback returns a
recorded response for a provider that closes stdout without a final
`agent_settled`. This also removes a latent early-return on the first
`agent_end` of an `auto_retry` run.

Kept #81's turn-schema/error handling intact; did not revert to returning on
`agent_end` only.

Regression test `reused_reader_does_not_replay_previous_turn` drives two turns
over one reader holding turn 1's full `turn_end{stop}` -> `agent_end` ->
`agent_settled` tail and asserts turn 2 delivers its own answer — the exact
gap in #81's tests, which never ran a second turn over a leftover tail.