[AiCaptain357]

Summary

When streaming execution fails, OmniRoute can overwrite an already captured outgoing request with a blank URL, response headers and the intermediate body. Failure persistence can also invent a provider request when execution stopped before any send.

Keep the existing prepared-request snapshot and pending URL/body on errors. Pass only a captured outgoing body to failure persistence, leaving the field absent when none exists. Capture deduplication also compares normalized headers and snapshots the header record, so a headers-only retry retains its own metadata. Equivalent capture hooks still coalesce. Success logging, masking, size limits and settled-dispatch guards stay intact; no capture history or raw-body copies are added.

Refs #18697, item 4. This fixes request diagnostics; the other three items remain separate.

Validation

• tests/unit/streaming-provider-request-error.test.ts has 16 behavioral cases using the real streaming pipeline, capture logger, pending tracker and persisted artifacts. Recorded RED checks demonstrate the overwrite, no-send fallback and headers-only retry defects.
• Cases cover HTTP/transport errors, pre-send failures, success, retries, concurrent isolation, project-header removal, mutable header records, equivalent header normalization, masking, bounds and late settled captures. Uncaptured translated bodies also stay out of error.requestBody.
• Targeted capture/lifetime/tool metadata/error privacy/attempt-persistence checks: 40 passed on release/v3.8.53 at 3d234932; all 16 regression cases were rerun after strengthening the case-sensitive header-value assertion.
• Scoped ESLint, Prettier and whitespace checks passed. Full unit/Vitest/coverage/build validation remains for CI; no live provider requests were made.

⚠️ base-red inherited: #17219.

[coderabbitai]

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

• Bug Fixes
  • Streaming failures now retain the latest captured provider request in error records and diagnostics, improving visibility into requests that were actually sent.
  • Failures that occur before an outgoing request is captured no longer include misleading provider-request details.
  • Request changes to headers are recognized during retries, and captured request details remain consistent across logging and failure records.
• Tests
  • Added coverage for streaming failures, retries, concurrent requests, and cases where request capture is unavailable.

Streaming provider request capture now detects header changes and stores a copy of each captured request. Streaming failure metadata and persisted logs use the latest captured request. When no request was captured, failure handling does not add provider-request details.

Changes: Provider Request Diagnostics

Layer / File(s)	Summary
Capture and compare prepared requests open-sse/utils/providerRequestLogging.ts, tests/unit/streaming-provider-request-error.test.ts	Capture comparison includes normalized headers, and capture stores a request copy with a separate headers object. Tests cover changed headers, concurrent captures, capture bounds, invalid headers, and equivalent fetch captures.
Use captured requests in streaming outcomes open-sse/handlers/chatCore/streamingResponse.ts, tests/unit/streaming-provider-request-error.test.ts, changelog.d/fixes/preserve-captured-provider-request.md	Pipeline errors record the response-started stage without request metadata. Failure logs use the latest captured request body. Tests cover pre-send and terminal failures, successful logging, retries, and persisted request data.

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

Change: Bug fix

Merge Risk: Minimal · up to 690a0

The changed streaming diagnostics appear ready to merge after normal checks; no actionable failure was established.

Security architecture risk: Low · up to 690a0

The change remains within existing request diagnostics and does not add provider access or privileges. Request isolation and existing logging protections are preserved. Risk is low because failure records now retain actual outgoing content, whose end-to-end privacy exposure was only partially assessed.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The supported exposure change is within an existing request's diagnostic state and retained failure records. The inspected paths do not establish a new cross-request, tenant, service, or credential-authority boundary crossing.

Security Findings and Attack Paths

• observed — The suspected unprotected upstreamRequestBody storage path was not confirmed. The sink extracts reasoning-effort metadata from that argument; its durable artifact builder receives protected request, response, error, and pipeline values rather than storing the raw argument as a separate body.

Trust Boundaries and Controls

• observed — Error-stage pending updates no longer substitute response-derived request metadata. Actual prepared-request captures still update pending state through its existing identity and payload-protection controls.

Resilience and Maintainability Implications

• inferred — A failed fallback can leave the latest captured request alongside the original selected failure. This is not retained as an introduced security concern: the base already updated the captured pipeline request during fallback while returning and persisting the original failure. The diagnostics should not be interpreted as a complete per-dispatch audit history.

Pre-merge checks: 6 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 6 passed

Check name	Status	Explanation
Title check		The title follows the required Conventional Commits format and accurately describes the streaming error fix.
Description check		The description clearly explains the change, related issue, validation results, tests, coverage behavior, and reviewer risk. It does not use every template heading, but it provides the required inform…
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.
Tests For Production Code		The PR changes production code in open-sse/handlers/chatCore/streamingResponse.ts and open-sse/utils/providerRequestLogging.ts. It adds tests/unit/streaming-provider-request-error.test.ts with 1…
No Ai Attribution		No AI attribution found. All three commits in the reviewed range have no trailers, and their messages contain neither a Co-Authored-By trailer nor a Generated with footer. The supplied PR body also co…

Full details: Description check

Explanation

The description clearly explains the change, related issue, validation results, tests, coverage behavior, and reviewer risk. It does not use every template heading, but it provides the required information in equivalent sections.

Full details: Tests For Production Code

Explanation

The PR changes production code in open-sse/handlers/chatCore/streamingResponse.ts and open-sse/utils/providerRequestLogging.ts. It adds tests/unit/streaming-provider-request-error.test.ts with 16 behavioral test cases covering the changed streaming request-capture behavior. The explicit test requirement is met.

Full details: No Ai Attribution

Explanation

No AI attribution found. All three commits in the reviewed range have no trailers, and their messages contain neither a Co-Authored-By trailer nor a Generated with footer. The supplied PR body also contains neither marker.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 6 functions across 3 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

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

Usage-based review receipt

• Mode: Continue automatically
• Reviewed files: 4
• Waived: $1.00 (charged $0.00)
• View usage details

Note

This review exceeded your plan’s limits and used usage-based reviews—free during trial, billed after paid activation unless disabled. Manage usage-based reviews.

Recent review info

Run configuration

• Source mode: Auto
• Repository: cheaper-inference/OmniRoute
• Branch: release/v3.8.53
• File: .coderabbit.yaml
• Commit: f000c11
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 669457aa-4d96-4cdd-a09a-c5a10b1da0aa

Commits

Reviewing files that changed from the base of the PR and between f000c11 and 690a01b.

Files selected for processing: 4

• changelog.d/fixes/preserve-captured-provider-request.md
• open-sse/handlers/chatCore/streamingResponse.ts
• open-sse/utils/providerRequestLogging.ts
• tests/unit/streaming-provider-request-error.test.ts

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