Skip to content

Fail send on settled Pi provider errors - #570

Merged
prathamdby merged 4 commits into
mainfrom
pd/fix/pi-terminal-errors-d906
Sep 5, 2026
Merged

Fail send on settled Pi provider errors#570
prathamdby merged 4 commits into
mainfrom
pd/fix/pi-terminal-errors-d906

Conversation

@prathamdby

@prathamdby prathamdby commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Summary

  • createPiSession.send fails after a settled unrecovered Pi provider error.
  • A later successful SDK retry still returns the recovered assistant text.
  • Public abort() stays agent.session_aborted when an earlier error turn remains.
  • Tool-round budget stops and idle timeouts keep their current outcomes.
  • Docs and the Pi session seam rule record this send boundary.

Details

piSessionImpl.ts keeps the latest assistant stopReason for each send. After session.prompt settles, an unrecovered error turn throws provider.request_failed before completion. A later non-error assistant turn clears that state.

Public abort wins over a retained error turn. A tool-budget abort does not become a provider outage. An idle timeout still throws pi.prompt_idle_timeout.

test/createPiSession.test.ts and test/piSession.seam.test.ts cover settled errors, retries, rejected prompts, cancel, successive sends, usage, and cleanup. test/fallbackClassification.test.ts keeps auth ineligible and rate-limit eligible.

Closes #558

Open in WebOpen in Cursor

PR Agent Description

PR Type

Bug fix, Tests, Documentation

Description

  • The change fails send with provider.request_failed when the SDK settles on an assistant error turn.
  • It keeps a later successful SDK retry as success and resets error state between sends.
  • It wraps a directly rejected prompt promise as provider.request_failed and preserves prior usage events.
  • It keeps public cancellation and idle timeout above provider errors and keeps tool-budget stops successful.
  • It extends fallback classification tests to map rate limits as eligible and auth or cancellation as ineligible.
  • It updates seam rules and guides to document the distinct failure, retry, and fallback contract.

Changes Diagram

flowchart LR
  A["Prompt send"] --> B["Track assistant error turn"]
  B --> C["Settled error fails send"]
  C --> D["Fallback classifies error"]
  B --> E["Successful retry returns text"]
Loading

Review map

  1. src/agent/runtime/piSessionImpl.ts: Core send contract decides failure versus silent success
  2. test/createPiSession.test.ts: Terminal error cases lock the new send behavior
  3. test/fallbackClassification.test.ts: Fallback mapping controls retry versus no-retry paths

- Add settled-error, retry, cancel, and tool-budget cases
- Add fallback classifier cases for auth and availability
- Raise provider.request_failed after the SDK settles
- Keep retry, cancel, timeout, and tool-budget outcomes
- Document the send outcome boundary
@zeus-review

zeus-review Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

PR Agent Review

Caution

Review did not finish. Run /review to try again.

@vercel

vercel Bot commented Sep 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
pr-agent Ready Ready Preview Sep 5, 2026 12:10pm UTC

- Keep the send outcome note outside the layout table
- Wrap a long session test line
@greptile-apps

greptile-apps Bot commented Sep 5, 2026

Copy link
Copy Markdown

Greptile Summary

This PR makes the shared Pi session boundary reject sends that settle on an unrecovered assistant provider-error turn while preserving successful SDK retries and the existing cancellation, idle-timeout, and tool-budget outcomes.

  • Tracks the latest assistant stop reason and converts terminal provider errors into provider.request_failed.
  • Preserves usage accounting and emits failure rather than completion for failed sends.
  • Adds coverage for retries, direct prompt rejection, cancellation precedence, tool budgets, successive sends, cleanup, and fallback classification.
  • Documents the resulting runtime and fallback contract.

Confidence Score: 5/5

The PR appears safe to merge; no actionable correctness, security, or repository-rule violation remains.

The implementation scopes provider-error state to each send, deliberately clears it after a successful SDK retry, and preserves the documented higher-priority cancellation, timeout, and tool-budget outcomes with focused regression coverage.

Important Files Changed

Filename Overview
src/agent/runtime/piSessionImpl.ts Adds per-send terminal provider-error tracking and explicit precedence among public cancellation, idle timeout, tool-budget termination, and provider failure.
test/createPiSession.test.ts Extensively exercises settled provider errors, SDK recovery, direct rejection, cancellation, usage preservation, cleanup, and state reset.
test/fallbackClassification.test.ts Locks rate-limit eligibility and auth, timeout, and cancellation ineligibility for fallback.
test/piSession.seam.test.ts Adds seam-level verification that a resolved SDK prompt ending in an assistant error rejects without emitting completion.
docs/operations.md Documents provider failure, retry, cancellation, tool-budget, and fallback behavior at the operational boundary.
docs/development.md Records the updated createPiSession.send contract for contributors.
.pr-agent/pi-session-seam.mdc Extends the binding Pi session seam rule with the distinct terminal-outcome contract.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Send prompt] --> B[Observe Pi turn events]
    B --> C{Latest assistant outcome}
    C -->|Provider error| D[Retain provider error]
    C -->|Successful retry| E[Clear retained error and keep response]
    B -->|Tool budget reached| F[Abort internally and preserve budget-stop outcome]
    B -->|Idle timeout| G[Abort internally and report idle timeout]
    A -->|Public abort| H[Report session aborted]
    D --> I{Prompt settled without higher-priority outcome?}
    I -->|Yes| J[Throw provider.request_failed]
    E --> K[Return assistant text]
Loading

Reviews (1): Last reviewed commit: "chore: format tests and clarify send doc..." | Re-trigger Greptile

@prathamdby

Copy link
Copy Markdown
Owner Author

/review

1 similar comment
@prathamdby

Copy link
Copy Markdown
Owner Author

/review

@prathamdby
prathamdby merged commit d8407d7 into main Sep 5, 2026
8 checks passed
@prathamdby
prathamdby deleted the pd/fix/pi-terminal-errors-d906 branch September 5, 2026 12:12
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.

[P1] Propagate terminal Pi provider errors instead of returning success

1 participant