fix: snapshot RequestType before closure to prevent pool-reuse corruption in streaming requests

[Pratham-Mishra04]

Summary

Under high concurrency, streaming requests were experiencing RequestType corruption in response extra fields. This happened because *BifrostRequest and *ChannelMessage objects are returned to their respective pools as soon as the stream channel is handed off, allowing a concurrent request to reuse and overwrite the RequestType field before the post-hook closure had a chance to read it.

Changes

• Snapshot req.RequestType into a local variable (shortCircuitRequestType in tryStreamRequest, attemptRequestType in requestWorker) before the post-hook closure is created, so the closure captures the correct value at the time of the request rather than reading from a potentially recycled object.

Type of change

• Bug fix
• Feature
• Refactor
• Documentation
• Chore/CI

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations
• Plugins
• UI (React)
• Docs

How to test

Reproduce by sending a high volume of concurrent streaming requests with different request types and verifying that the RequestType field in response extra fields consistently matches the originating request.

go version
go test ./...

Screenshots/Recordings

Breaking changes

• Yes
• No

Related issues

Security considerations

No security implications.

Checklist

• I read docs/contributing/README.md and followed the guidelines
• I added/updated tests where appropriate
• I updated documentation where needed
• I verified builds succeed (Go and UI)
• I verified the CI pipeline passes locally if applicable

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.

❌ Pratham-Mishra04
❌ akshaydeo
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[Pratham-Mishra04]

• docs: add default provider resolution docs #3069 : 2 dependent PRs (#3076 , #3079 )
• feat: auto-resolve provider when no provider prefix is given on integration routes #3068
• feat: auto-resolve provider when no provider prefix is given on inference routes #3067
• feat: add opt-in per-request overrides for content logging and raw request/response visibility #3066
• fix: snapshot RequestType before closure to prevent pool-reuse corruption in streaming requests #3065 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6e4a4d9e-fc10-4807-8a80-77340ee3ad71

📥 Commits

Reviewing files that changed from the base of the PR and between a45bd43 and a9bd949.

📒 Files selected for processing (2)

• core/bifrost.go
• core/changelog.md

✅ Files skipped from review due to trivial changes (1)

• core/changelog.md

---

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes
  • Fixed request-type corruption for streaming responses under high concurrency and retries so streamed chunks and errors are annotated with the correct request type.
  • Restored fallback to the network configuration base URL when no URL is provided for Ollama and SGL, improving connection reliability.

Walkthrough

A concurrency bug fix snapshots RequestType into per-closure local variables before launching asynchronous post-hook closures, preventing pooled *BifrostRequest reuse from overwriting request-type annotations on streaming post-hook chunks and errors.

Changes

Cohort / File(s)	Summary
Streaming post-hook concurrency fix core/bifrost.go	Capture RequestType into local snapshot variables (shortCircuitRequestType, attemptRequestType) before creating async post-hook runner closures; replace uses of req.RequestType inside those closures with the snapshots so post-hook chunk/error metadata remains consistent under concurrency.
Changelog documentation core/changelog.md	Add changelog entry documenting the fix for response extra-fields request-type corruption during high-concurrency streaming requests and restore a related network base-URL fallback fix entry.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I freeze the type before the race begins,
So pooled requests can't change their skins,
Closures hold truth in a quiet snap,
No scrambled tags, no racing gap,
Streamed chunks rest safe in tins.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main fix: snapshotting RequestType before closure to prevent pool-reuse corruption in streaming requests, which directly corresponds to the core changes made in the PR.
Description check	✅ Passed	The description includes all major template sections: a clear summary of the problem, detailed changes explaining the fix, correct type and affected areas checkboxes, testing instructions, breaking changes confirmation, and security considerations. The description is well-structured and provides sufficient context.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 04-26-fix_pool_req_reuse_fixes_for_streaming_extra_fields

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.4)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

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

[github-actions]

🧪 Test Suite Available

This PR can be tested by a repository admin.

Run tests for PR #3065

[greptile-apps]

Confidence Score: 5/5

Safe to merge — targeted, minimal fix for a real concurrency bug with no behavioral changes outside the race window.

Change is small and focused: two local-variable snapshots mirroring the already-present attemptResolvedModel pattern. The non-streaming path is unaffected as it is fully synchronous. No new logic, no API changes, and no regressions identified.

No files require special attention.

Important Files Changed

Filename	Overview
core/bifrost.go	Snapshots req.RequestType into local variables (shortCircuitRequestType in tryStreamRequest, attemptRequestType in requestWorker) before closure creation, preventing pool-reuse data races on streaming requests.
core/changelog.md	Adds changelog entry documenting the streaming request-type corruption fix.

_{Reviews (3): Last reviewed commit: "Merge branch 'main' into 04-26-fix_pool_..." | Re-trigger Greptile}

[coderabbitai]

🧹 Nitpick comments (1)

core/bifrost.go (1)

5702-5727: Please add a regression test for this pooled-request capture path.

This closure fix looks right, but it guards a subtle concurrency bug. A streaming test that interleaves different RequestType values across concurrent attempts would make this much harder to regress.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/bifrost.go` around lines 5702 - 5727, Add a regression test that
reproduces the pooled-request concurrency scenario around tryStreamRequest and
postHookRunner to ensure the captured attemptRequestType doesn't bleed between
concurrent streaming attempts: write a streaming test that starts multiple
overlapping attempts with different RequestType values, forces the code path
that releases and reuses the pooled *ChannelMessage (so tryStreamRequest
interleaves attempts), and verifies via PopulateExtraFields/RunPostLLMHooks
results and errors (and IsFinalChunk/drainAndAttachPluginLogs behavior) that
each attempt's RequestType/provider/model remains the per-attempt snapshot (no
cross-attempt contamination). Ensure the test asserts for both successful chunks
and errors and fails if any response/error shows the wrong RequestType.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@core/bifrost.go`:
- Around line 5702-5727: Add a regression test that reproduces the
pooled-request concurrency scenario around tryStreamRequest and postHookRunner
to ensure the captured attemptRequestType doesn't bleed between concurrent
streaming attempts: write a streaming test that starts multiple overlapping
attempts with different RequestType values, forces the code path that releases
and reuses the pooled *ChannelMessage (so tryStreamRequest interleaves
attempts), and verifies via PopulateExtraFields/RunPostLLMHooks results and
errors (and IsFinalChunk/drainAndAttachPluginLogs behavior) that each attempt's
RequestType/provider/model remains the per-attempt snapshot (no cross-attempt
contamination). Ensure the test asserts for both successful chunks and errors
and fails if any response/error shows the wrong RequestType.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6e3d02e3-0961-484b-8535-757ae6a7eeb7

📥 Commits

Reviewing files that changed from the base of the PR and between 20fbc0a and cc6d227.

📒 Files selected for processing (2)

• core/bifrost.go
• core/changelog.md

[akshaydeo]

Merge activity

• Apr 28, 6:57 AM UTC: A user started a stack merge that includes this pull request via Graphite.
• Apr 28, 6:57 AM UTC: Graphite couldn't merge this PR because it had merge conflicts.
• Apr 28, 7:01 AM UTC: A user started a stack merge that includes this pull request via Graphite.
• Apr 28, 7:02 AM UTC: @akshaydeo merged this pull request with Graphite.