Skip to content

[fix] Keep Pi retries silent and report failed turns - #6953

Merged
mmabrouk merged 2 commits into
release/v0.119.0from
agent/fix-pi-silent-retries
Sep 18, 2026
Merged

mmabrouk merged 2 commits into
release/v0.119.0from
agent/fix-pi-silent-retries

Conversation

@mmabrouk

@mmabrouk mmabrouk commented Sep 18, 2026

Copy link
Copy Markdown
Member

Pi sessions could stop after displaying “Retrying (attempt 1/3...)” and be recorded as completed, even though the model request failed midway through work. The adapter treated agent_end, which ends one attempt, as completion of the user turn. The runner also skipped transcript error recovery whenever the agent had already produced text or used tools.

This change waits for Pi 0.85.1's agent_settled event before completing the prompt, publishing usage, or closing the root trace. Retry events produce no assistant text. Successful recovery continues the same prompt; exhausted recovery rejects it and records an error ending, including after partial work. Existing conversation and tool records remain available. A terminal WebSocket failure displays: “The agent lost its connection while working and could not recover. You can continue from here.”

Transport diagnostics that Pi supplies now reach trace events through a bounded allowlist, without provider request bodies, headers, or stacks. This does not establish or fix the original socket disconnect. Pi's loss of close metadata after an opaque socket error remains an upstream limitation. This change also does not add a new Continue button or deduplicate text that the model regenerates after a partially streamed response.

Validation

Live QA on Hetzner with real Pi, pi-acp, sandbox-agent and OpenAI Codex requests found and fixed an error that the initial unit tests missed: a plain Error became a generic internal error at the ACP RPC boundary. The follow-up uses the SDK's RequestError and covers the actual RPC boundary.

On final head 78935f35e3ef027ef88ada5da069e9cf5bf2f813, in an isolated runner:

  • Closed the real model WebSocket after a completed tool. HTTP fallback recovered and finished with no retry text.
  • Repeated the same socket cut against the current oss-team image. It reproduced the original false success with “Retrying...” as its only answer.
  • Blocked subsequent provider fetches until recovery was exhausted. The fixed build retained prior tool events, emitted the intended failure message, and ended with stopReason=error and ok=false.
  • Cancelled during recovery. The fixed build settled as cancelled in 790 ms.
  • Sent another message in the same warm session after recovery. It successfully read the existing file.
  • Recovered after partial text and a file append. The completed write was not repeated: a separate read confirmed exactly one line.

All 3,181 runner unit tests, typecheck, and diff checks pass. The extension build passed before the RPC follow-up; its source was unchanged by that follow-up. Relevant deployed source and dependency files were checked byte-for-byte against the final PR head. The shared oss-team containers were never replaced or restarted; isolated test containers were removed.

This is live backend coverage, not a completed browser acceptance pass. Authenticated UI rendering and application database ingestion remain unverified. The physical cause of the original disconnect also remains unknown.

What to QA

  • In a Pi chat session, inject a transient provider disconnect after some tool work. The turn should stay running, show no retry notice, and continue when recovery succeeds.
  • Exhaust provider retries. The session should retain prior work and show a failed turn instead of a normal completion. Sending a continuation should start another turn.
  • Stop during retry backoff. The turn should settle as cancelled rather than report the preceding provider failure.
  • Check a recovered turn's trace: one root, all model attempts, and accumulated usage. No retry notices should appear in stored conversation records.

The dependency patch relies on Pi's pinned agent_settled contract. Review pi-acp@0.0.29.patch together with the adapter lifecycle tests when upgrading either package.

@coderabbitai

coderabbitai Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)
  • services/runner/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: bdcb435d-e652-45a6-8527-7769a4cf1def

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: a0ee1343-5787-4ccb-b8d4-fbe84f24c37b

📥 Commits

Reviewing files that changed from the base of the PR and between 7c92bdb and 78935f3.

⛔ Files ignored due to path filters (1)
  • services/runner/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (3)
  • services/runner/patches/pi-acp@0.0.29.patch
  • services/runner/src/engines/sandbox_agent/errors.ts
  • services/runner/tests/unit/pi-acp-retry.test.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.


📝 Summary

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of interrupted or failed runs, including failures after partial text or tool activity.
    • Connection interruptions now show clearer “lost connection while working” messages.
    • Recovered retry attempts are no longer incorrectly reported as failures.
    • Cancellation and queued requests behave more reliably during automatic retries.
    • Failed runs are consistently marked as errors in activity and usage reporting.
  • Reliability

    • Partial progress and session continuity are better preserved during interrupted runs.
    • Usage and tracing updates now complete reliably after a run settles.

Walkthrough

Pi turns now settle at agent_settled. The runner records errors after partial output, classifies connection failures, and updates OTel tracing. Tests cover retries, cancellation, recovery, usage ordering, and trace serialization.

Changes

Pi turn lifecycle and configuration

Layer / File(s) Summary
Adapter settlement lifecycle
services/runner/patches/pi-acp@0.0.29.patch, services/runner/src/extensions/agenta.ts, services/runner/src/engines/sandbox_agent/environment.ts, services/runner/src/engines/sandbox_agent/usage.ts, services/runner/tests/unit/pi-acp-retry.test.ts, services/runner/tests/unit/extension-tools.test.ts
Pi prompts now resolve or reject at agent_settled. Retry events no longer emit assistant text. Skill and session directories can come from environment variables. Usage publication and trace flushing use the settled event.
Transcript-driven error recovery
services/runner/src/engines/sandbox_agent/pi-error.ts, services/runner/src/engines/sandbox_agent/run-turn.ts, services/runner/src/engines/sandbox_agent/errors.ts, services/runner/tests/unit/daytona-transcript-recovery.test.ts, services/runner/tests/unit/sandbox-agent-pi-error.test.ts, services/runner/tests/unit/silent-turn-contract.test.ts
Recovery evaluates the newest assistant transcript record after the turn cursor, including turns with partial text or tool calls. Later successful records supersede earlier errors. Connection failures use a recovery-oriented runner_error message.
Settled-turn tracing
services/runner/src/tracing/otel.ts, services/runner/tests/unit/otel-trace-serialization.test.ts
Tracing stores final assistant messages at agent_end and closes the agent span at agent_settled. Retries do not create duplicate agent spans. Bounded provider transport diagnostics are recorded as span events.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~30 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Runner
  participant PiAdapter
  participant Transcript
  participant OTel
  Runner->>PiAdapter: start prompt
  PiAdapter-->>Runner: partial messages and retry events
  PiAdapter-->>Runner: agent_settled
  Runner->>Transcript: inspect latest assistant record
  Transcript-->>Runner: success or provider error
  Runner->>OTel: close settled turn span
Loading

Possibly related PRs

  • Agenta-AI/agenta#6137: Adds the earlier cursor-based Pi transcript recovery that this change broadens to cover partial output and tool work.

Merge Risk: ⚪ Minimal · up to 78935

No actionable risk remains from the reviewed changes; the reported error-classification regression is not reachable.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 62.50% which is sufficient. The required threshold is 60.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 12 files. (1 skipped: 1 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main changes: silent Pi retries and failed-turn reporting.
Description check ✅ Passed The description directly explains the retry lifecycle changes, failed-turn handling, diagnostics, validation, and known limitations.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Railway Preview Environment

Status Destroyed (PR closed)

Updated at 2026-09-18T15:29:54.532Z

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: 861ee753-1456-44cb-b2e0-56015e08e66b

📥 Commits

Reviewing files that changed from the base of the PR and between 20c578a and 7c92bdb.

⛔ Files ignored due to path filters (1)
  • services/runner/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (14)
  • services/runner/patches/pi-acp@0.0.29.patch
  • services/runner/src/engines/sandbox_agent/environment.ts
  • services/runner/src/engines/sandbox_agent/errors.ts
  • services/runner/src/engines/sandbox_agent/pi-error.ts
  • services/runner/src/engines/sandbox_agent/run-turn.ts
  • services/runner/src/engines/sandbox_agent/usage.ts
  • services/runner/src/extensions/agenta.ts
  • services/runner/src/tracing/otel.ts
  • services/runner/tests/unit/daytona-transcript-recovery.test.ts
  • services/runner/tests/unit/extension-tools.test.ts
  • services/runner/tests/unit/otel-trace-serialization.test.ts
  • services/runner/tests/unit/pi-acp-retry.test.ts
  • services/runner/tests/unit/sandbox-agent-pi-error.test.ts
  • services/runner/tests/unit/silent-turn-contract.test.ts
💤 Files with no reviewable changes (1)
  • services/runner/tests/unit/sandbox-agent-pi-error.test.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.

Comment thread services/runner/src/tracing/otel.ts
@mmabrouk
mmabrouk changed the base branch from main to release/v0.119.0 September 18, 2026 15:29
@mmabrouk
mmabrouk merged commit 1ea437a into release/v0.119.0 Sep 18, 2026
66 of 67 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant