[StiensWout]

ACP model rejection errors were hidden by generic session-open and turn-start wrappers. Invalid-parameter failures at the model-write boundary now retain a fixed message naming the selected model and explaining how to retry. Native error text stays in the cause; authentication, transport, and other configuration failures retain their handling.

This preserves @StiensWout’s implementation from #19699, resolves the conflicts by merging current main, and consolidates the regression coverage into the existing replay suite. Fixes #18586.

Verification

• Three real-stdio replay cases cover rejection during open, rejection after a completed turn, and unchanged authentication failure. The original four cases in a one-off file became three parameterized cases in the existing replay fixture suite, removing 83 lines. Removing curated startup-message precedence reproduces the generic error.
• 89 focused tests passed across orchestration, ACP runtime, Grok selection, and failure/error boundaries. Scoped server typecheck, targeted lint and formatting, and diff checks passed. One existing registry test timed out during concurrent resource-heavy checks, then passed in the complete 46-test sequential rerun without code or timeout changes.
• One full adversarial review round on the entire diff against main, independently by Claude Opus 5.5 and Codex GPT-6-Sol: no confirmed findings. The implementation was separately assessed by GPT-6.1-Sol after the Astra test pass.
• In the isolated Linux web app, a local ACP stdio fixture rejected an advertised model with -32602. Verified opening rejection, successful retry with another model, and rejection when switching the same session after a completed response. The earlier completed response remained intact. Captures use the reviewed bda5530; the baseline uses the exact three production files from main ab6be13, with the same fixture, model, prompt and viewport.

Before: generic session error	After: rejected model and retry advice

After a completed turn, switching to the rejected model:

Limits

The live run used a protocol fixture, not fx or Cline with a real account. Resume shares the model configuration and error-delivery paths but was not separately exercised live. Providers returning rejection codes other than -32602 retain existing handling. macOS, Windows, iOS, Android, Electron desktop, and relay/tunnel connections were not verified here.

Prepared in T3 Code using Codex GPT-6-Astra (tests), GPT-6.1-Sol (implementation assessment and integration), GPT-6-Sol and Claude Opus 5.5 (independent reviews).

[macroscopeapp]

Approvability

Verdict: Approved at 84a9af6

Macroscope's review found this PR approvable — This localized ACP error-handling fix replaces generic session-open/turn-start messages with a curated model-rejection explanation while preserving existing handling for authentication and transport failures. The accompanying replay tests cover model rejection during opening and switching without changing product defaults or APIs.

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

[coderabbitai]

Warning

Review limit reached

Only developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing.

Next included review available in 10 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: 9fb9d8e4-384a-4a14-aeba-2fae1d582097

📥 Commits

Reviewing files that changed from the base of the PR and between bda5530 and 508248a.

📒 Files selected for processing: 4

• apps/server/src/orchestration-v2/ProviderTurnStartService.ts
• apps/server/src/orchestration-v2/testkit/OrchestratorReplayFixtures.integration.test.ts
• packages/provider-acp/src/server/adapter.ts
• packages/provider-core/src/server/failure.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: 55e09f7b-0c3d-457d-b096-883c2dc1e0c1

📥 Commits

Reviewing files that changed from the base of the PR and between 345a1a8 and bda5530.

📒 Files selected for processing: 4

• apps/server/src/orchestration-v2/ProviderTurnStartService.ts
• apps/server/src/orchestration-v2/testkit/OrchestratorReplayFixtures.integration.test.ts
• packages/provider-acp/src/server/adapter.ts
• packages/provider-core/src/server/failure.ts

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

📝 Walkthrough

The ACP adapter classifies invalid-parameter model-selection errors. Provider failure handling preserves known messages, and orchestration uses them when settling provider-start failures. Integration tests cover model-write failures during session opening, first-turn switching, and authentication.

Changes: ACP model-selection failure reporting

Layer / File(s)	Summary
Classify and curate model-selection failures packages/provider-acp/src/server/adapter.ts, packages/provider-core/src/server/failure.ts	The ACP adapter maps request errors with code -32602 to AcpModelSelectionError. Failure extraction distinguishes known domain messages from provider categories, and makeProviderFailure prioritizes explicit and known messages.
Surface and verify failure messages apps/server/src/orchestration-v2/ProviderTurnStartService.ts, apps/server/src/orchestration-v2/testkit/OrchestratorReplayFixtures.integration.test.ts	Provider-start failure settlement prefers a known message. Replay tests assert outcomes for model-write rejections during opening, first-turn switching, and authentication.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to bda55

The reviewed change is mergeable after normal checks; no unresolved failure in model-rejection reporting was established.

Security architecture risk: ⚪ Minimal · up to bda55

The change improves rejection messages without adding access, privileges, or execution paths. Existing failure handling and message sanitization remain in place. No material security regression was identified in the reviewed changes.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed exposure is the failure explanation associated with an existing provider session or turn. The inspected comparison adds no route, credential capability, tenant selector, or execution authority; the public-entrypoint signal resolves to an internal error declaration.

Trust Boundaries and Controls

• observed — A provider-controlled invalid-parameter response can trigger classification, but its native text does not become the curated explanation. The adapter constructs that explanation locally, orchestration gives it precedence, and provider-core sanitizes the resulting failure message.

Resilience and Maintainability Implications

• inferred — The new error mapping preserves the existing failure-containment sequence: local selection is invalidated before configuration, and the requested selection is committed only after configuration succeeds. Startup settlement still returns failures for scheduled retry or records terminal failure according to the existing willRetry branch. Provider-side rollback after partial application remains unproven and is not a new guarantee of this change.

Pre-merge checks: 4

Check name	Status	Explanation
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.
Title check		The title clearly and concisely describes the main change: improving error messages for rejected ACP model selections.
Description check		The description clearly explains the problem, implementation, scope, verification results, known limits, and agent usage. It references the related issue and provides detailed evidence, although it do…

Full details: Description check

Explanation

The description clearly explains the problem, implementation, scope, verification results, known limits, and agent usage. It references the related issue and provides detailed evidence, although it does not use every template heading or state explicit maintainer approval.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

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

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

[juliusmarminge]

Taking over this one. I will resolve the conflicts against current main, keep the minimum behavioral coverage, and rebuild the model rejection error path before pushing back here.

Dismissing prior approval to re-evaluate bda5530

[juliusmarminge]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Dismissing prior approval to re-evaluate 84a9af6