fix(pi): stop marking recovered turns as failed - #24805

Approvability

Verdict: Approved at 5d4c49e

Macroscope's review found this PR approvable — This is a focused Pi adapter bug fix that separates transient message errors from terminal failures so recovered continuations persist as completed. It changes one production adapter and adds targeted unit and orchestration regression coverage without altering schemas, defaults, infrastructure, or static-analysis settings.

^{You can add or adjust custom eligibility rules. Learn more.}

No actionable comments were generated in the recent review.

Merge Risk: Minimal · up to e77d6

This fix stops Pi turns that recovered after an intermediate error from being recorded as failed. Cancellation and unrecovered errors keep their existing behavior. No merge-blocking risk was identified.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title "fix(pi): stop marking recovered turns as failed" follows conventional commit format and matches the main change. The adapter now clears a stale assistant message error after a successful re…
Description check		The description covers the problem, the change, the scope justification, and detailed verification. It follows the template's Problem, Change, Scope, and Verification sections. It states the bug, expl…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title "fix(pi): stop marking recovered turns as failed" follows conventional commit format and matches the main change. The adapter now clears a stale assistant message error after a successful response, so recovered turns no longer persist as failed. The title is short, specific, and states the primary bug fix from the developer's perspective.

Full details: Description check

Explanation

The description covers the problem, the change, the scope justification, and detailed verification. It follows the template's Problem, Change, Scope, and Verification sections. It states the bug, explains the fix at the adapter level, cites the small obvious-bug exception with a link, and lists focused test commands, results, and unverified areas. It also names the agent and model used, as the template requests. The Problem section is folded into the opening paragraph rather than under a separate heading, which is a minor deviation.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: d899bdd
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: 17267137-9b3c-48a4-b196-13c5a18c72c5

Commits

Reviewing files that changed from the base of the PR and between 5d4c49e and e77d614.

Files selected for processing: 2

• packages/provider-pi/src/server/adapter.test.ts
• packages/provider-pi/src/server/adapter.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Verified in a real dev environment

verified as-is at 5d4c49e243e099faaede49a7981dc1bd06cfcc73. reproduced the bug first on github/main at 32ecd71726 in /home/user/main, then repeated it on the exact pr head. one dev server ran at a time, with isolated state. no source changes or pushes.

used repository playwright with /usr/bin/google-chrome. created the project and threads through the web app. codex gpt-6.1-sol completed a baseline turn on both revisions. the changed path used real pi 1.4.0 with cliproxyapi/gpt-6.1-sol through this vm's proxy using its chat-completions api.

reproduction: disable pi's automatic retries, start a long response, click "steer message" with Reply exactly RECOVERY_OK. Do not use tools or change files., then disconnect only pi's active tcp socket with sudo -n ss -K dst 127.0.0.1 dport = 28317 sport = <pi-client-port>. this induces a real connection failure without replacing the provider, injecting events, or using replay data. both native pi sessions recorded stopReason=error with Connection error., followed by stopReason=stop with RECOVERY_OK.

read-only sqlite inspection after the ui completed:

                         main                 pr head
assistant response       RECOVERY_OK          RECOVERY_OK
run / attempt / turn     failed               completed
provider session         error                ready
lastError                Connection error.    null
persisted error items    1                    0

neighboring checks on the pr: disconnecting pi without a steer still failed; "stop generation" persisted interrupted; another message after stop completed with AFTER_STOP_OK. reloading preserved the recovered response and clean state.

blacksmith vp run check:changed a96cf0818eff4a113c3ff52ae702683e51a5803b passed format, lint, knip, graphql, five affected typechecks, and all 105 tests in the two affected test files. the explicit base was needed because the worker starts with shallow history. two existing unused-variable lint warnings remain.

screenshots use matching prompts and response data. the recording shows the steer and successful continuation; loading screens were removed. uploaded images, video playback, and seeking were checked in chrome.

Not verified

• the original "This operation was aborted" trigger was not reproduced. the live reproduction exercised the same stale-message-error bookkeeping with a controlled connection failure.
• pi's responses api could not complete through this proxy, returning empty-response errors; live pi verification used chat completions.
• native ios/android, electron, macos/windows, remote relay/tunnel, and live compaction/exhausted-retry recovery were not exercised on this linux vm. focused existing tests cover the latter terminal semantics.

Following up on this fix. I am still encountering the reported bug. Could a maintainer take another look?

A fresh source review against main at 7e565c510d6dfaef2e1fd97bf9d89a97ce9cece7 confirms that the stale-error path is still present: an assistant message_end with stopReason: error sets turn.failure, and a later successful assistant message does not clear it. Without an explicit successful retry or retrying compaction event, finalization still marks the recovered turn failed.

Re-reviewed PR head 5d4c49e243e099faaede49a7981dc1bd06cfcc73 and found no new correctness issues. The patch clears recovered assistant-message failures while retaining terminal failures and cancellation semantics.

Fresh verification on the unchanged PR head:

• PASS: pnpm exec vp test run packages/provider-pi/src/server/adapter.test.ts apps/server/src/orchestration-v2/testkit/PiReplayFixtures.integration.test.ts -t 'PiAdapterV2|PiRpc|recovered Pi message error', exit 0. Both test files passed, 97 tests passed and 8 were skipped by the name filter.
• PASS: git diff --check, exit 0.

This follow-up includes source review and focused tests, not a new live-provider or UI reproduction. The earlier live verification in #24805 (comment) remains relevant.

@juliusmarminge, since you've worked on Pi's compaction and steering behavior, could you take a look when you have time?