[yan-ad]

Problem

Provider errors such as No tool output found for function call call_119 are currently returned from session/prompt as an ACP internal error. That ends the interaction as a protocol failure instead of reporting a failed turn that the user can recover from.

Changes

• Report provider stream failures through an agent message and return end_turn, keeping the session available for another prompt.
• Mark unfinished tool calls as failed, notify the client, and persist their error results without changing completed tools.
• Finalize pending tool results on cancellation or stream failure so persisted history does not leave running tools behind.
• Keep genuine notification and persistence failures as errors.
• Reserve original Responses call IDs before renaming duplicates, preventing generated suffixes from colliding with IDs already in history.

Regression coverage

Tests cover failures before any tool runs and while a tool is pending, including client-visible notifications, persisted results, preservation of completed tools, and a successful next turn on the same connection. A serializer regression covers ID suffix collisions with out-of-order concurrent outputs.

Validation

On this branch against current upstream main:

• cargo test acp::service:: -- --test-threads=1 — 58 passed
• cargo test aisdk::providers::openai::tests:: -- --test-threads=1 — 53 passed
• cargo fmt --check
• git diff HEAD^ --check
• bash scripts/check-aisdk-boundary.sh

Also synced and pushed to yan-ad/crabcode:yan-flavor as b802f87; the flavor branch passed 72 ACP tests, 53 OpenAI tests, and all 8 ACP stdio tests. This PR contains only the fix, not the unrelated flavor changes.

[Blankeos]

fix/acp-provider-error-recovery → main
Model: claude-sonnet-5-5, effort unknown

Confidence: 4/5

The fix does what it says, in a real ACP process as well as in unit tests. The remaining doubts are the raw error text now landing in chat history and the merge conflict with #67.
Checks run:

• cargo fmt --check: clean.
• cargo build: ok.
• bash scripts/check-aisdk-boundary.sh: ok.
• cargo test (whole bin suite): 1874 passed, 2 failed. Neither failure touches this PR.
  • ui::components::chat::tests::test_edit_tool_renders_codex_style_diff_summary expects a relative path but gets /Users/carlo/Desktop/Projects/crabcode/README.md, so it depends on the machine's cwd.
  • model::discovery::tests::custom_endpoint_failure_preserves_manual_models_and_is_cached failed in the full run and passed when run alone, so it is flaky under parallel load.
• Targeted runs, all green: acp::service:: and aisdk::providers::openai (111 passed), and --test acp_stdio (7 passed).
• The new provider_failure_* tests fail if I put the old Err(internal_error_with(..)) return back, so they do exercise the change.
• cargo clippy --all-targets: nothing reported on lines this PR adds. The pre-existing repo-wide clippy errors are untouched.
• End-to-end on the real binary: crabcode acp over stdio against a local mock OpenAI-compatible server, with a temp XDG_*, CRABCODE_TEST_MODE=1 and no real credentials.
  • Turn 1: the mock returns HTTP 400 "No tool output found for function call call_119". session/prompt returns {"stopReason":"end_turn"} (not an RPC error) and the client gets an agent_message_chunk starting "The AI request failed: ...".
  • Turn 2, same session: succeeds ("Recovered: the next turn works.").
  • The mock saw 2 requests, and request 2 contained the failure text, so it is replayed to the model.
• Screenshots do not apply. There is no TUI or CLI visual change; the change is ACP protocol behaviour, covered by the transcript above.

Confidence bumps:

• Reproduce with a real OpenAI Responses API session hitting the call_119 mismatch. This needs real credentials and a paid call, which I did not make. Raises it to 4.5/5.
• Check the behaviour in an actual ACP client (Zed or similar) to see how it renders an end_turn that carries an error message. I have no such client here.

Summary

In receive_prompt_stream, a provider Failed chunk used to be recorded and the stream kept being read. At the end the whole prompt returned an ACP internal error, so a provider 400 looked like a protocol failure and the client lost the turn. Now:

• The loop breaks on Failed.
• Any tool call still running/pending is marked error, and the client is sent a failed tool_call_update.
• The error is appended to the assistant message and streamed as an agent_message_chunk.
• The prompt returns end_turn, and the session status ends as Idle instead of Failed.
• Cancellation also finalizes dangling tool parts, so persisted history has no running tools. That is what prevents the next request from hitting "No tool output found".

dedupe_responses_call_ids in the OpenAI provider is also reworked. It reserves every call_id already present, so a generated _dupN suffix can no longer collide with an existing id such as call_1_dup2. The call and its output share one replacement via a (call_id, occurrence) map. A test with out-of-order concurrent outputs is added.

Two new ACP tests (no tools, and with tools plus a completed tool preserved) check notifications, persisted tool results and a following successful turn. One new serializer test covers the id collision.

Findings:

1. The full provider error string is now shown to the user and written into session history. It is the verbose internal diagnostic (provider_step_error step=1 messages=2 tools=20 ... tool_names=[...] error=...). In my run request 2 carried The AI request failed... back to the model as assistant text. That is noisy for users and wastes context. Consider showing a short message to the user and keeping the long diagnostic out of persisted assistant content.
2. A provider failure no longer sets SessionStatus::Failed. The status is now Idle, and only an internal persistence or notification error gives Failed. This is intended and tested, but anything that reads Failed (session lists, UI) will no longer see provider errors.
3. After a failed turn the prompt still runs maybe_auto_compact_session, which the old Err path skipped. If the session is near the compaction threshold, compaction would call the same failing provider and could surface an error from session/prompt again. This is an edge case, and a stop_reason != Cancelled && failed.is_none() guard would remove it.
4. Breaking on Failed drops chunks that arrive afterwards (Metrics, Usage). For the current producers Failed is terminal, so I don't expect a problem. A late usage chunk for the failed step would not be counted.

Map

flowchart LR
  P[provider stream: ChunkMessage::Failed] --> R[acp/service.rs receive_prompt_stream]
  R -->|break + mark tools error| T[send_tool_result: failed updates]
  R -->|append error text| M[agent_message_chunk]
  R -->|persist + status Idle| S[session manager / HistoryDAO]
  R -->|return end_turn| C[ACP client]
  O[aisdk/providers/openai.rs dedupe_responses_call_ids] -->|reserved ids| B[build_openai_messages]

Loading

Start in src/acp/service.rs around line 1715 (the Failed arm) and 1774 (the new finalization block).

Regressions

• No test regression from this PR.
• Behaviour change: provider errors are now reported as end_turn plus text instead of an RPC error (see findings 1-3). ACP clients that relied on the error response to detect failure will now only see the message text.
• The dedupe_responses_call_ids rewrite produces the same ids as before for the old simple duplicate case (call_1 → call_1_dup2). Only collision cases change. The two preserved tests still pass.
• Conflicts with main: none, it merges cleanly.
• Overlap with other open PRs: refactor(acp): split oversized service into focused modules #67 (splits src/acp/service.rs into service/* modules) conflicts, because refactor(acp): split oversized service into focused modules #67 deletes service.rs and this PR modifies it. Whichever lands second needs to re-apply the change in service/stream.rs or prompt.rs. fix(acp): trigger automatic compaction from provider context usage #64, feat(cli): add --auto permission mode #69 and feat(auth): add Google Antigravity browser OAuth #70 also touch service.rs, and they merge cleanly with this branch.

Migrations

None.

Before merging

• Decide whether to keep the verbose provider diagnostic in chat and persisted history (finding 1)
• Optionally skip auto-compaction after a failed turn (finding 3)
• Sequence with refactor(acp): split oversized service into focused modules #67 (service.rs split) and port the change to the new module layout if refactor(acp): split oversized service into focused modules #67 lands first
• The two unrelated failing tests (test_edit_tool_renders_codex_style_diff_summary, flaky custom_endpoint_failure_...) are not caused by this PR