encrypted content bug fix - #5982
Merged
Merged
Conversation
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change updates reasoning replay conversion for Anthropic, Cohere, Gemini, and Bedrock. It adds regression coverage, OpenCode Anthropic CLI scenarios, contextual rate-limit detection, report cleanup, static provider-harness publication, credential redaction, and workflow version validation. ChangesProvider reasoning replay
CLI harness and reporting
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLIHarness
participant OpenCodeAnthropic
participant Bifrost
participant ProviderConverter
participant ReportViewer
CLIHarness->>OpenCodeAnthropic: run reasoning-tool-replay
OpenCodeAnthropic->>Bifrost: send Anthropic Messages requests
Bifrost->>ProviderConverter: convert reasoning history and signatures
ProviderConverter-->>Bifrost: return replayable reasoning blocks
Bifrost-->>OpenCodeAnthropic: return assistant responses
OpenCodeAnthropic-->>CLIHarness: emit JSON assistant text
CLIHarness->>ReportViewer: render static harness report
ReportViewer-->>CLIHarness: write redacted HTML report
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
akshaydeo
marked this pull request as ready for review
August 8, 2026 20:49
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
18 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/providers/bedrock/reasoningreplayaudit_test.go`:
- Around line 143-152: Update the test loop over blocks to count each block with
Type "thinking" that is inspected, then require that count is non-zero after the
loop. Keep the existing thinking-key validation and ensure the test fails when
no thinking blocks are emitted, even if other blocks are present.
In `@core/providers/cohere/responses.go`:
- Around line 1697-1708: The Cohere ingress conversion must recognize the
“[ENCRYPTED_REASONING: ...]” marker before mapping a thinking block to reasoning
text. Update the thinking-block conversion to extract the marker into
ResponsesReasoning.EncryptedContent and exclude it from ordinary reasoning
content, while preserving normal thinking blocks; add a round-trip test covering
the egress marker and ingress restoration.
In `@core/providers/gemini/responses.go`:
- Around line 2700-2725: Update the Gemini response and input conversion flows
around the function-call look-ahead and standalone reasoning handling to
preserve reasoning messages that contain text. Apply positional look-ahead only
when the signed reasoning message is signature-only; process text-bearing
reasoning messages normally so their text remains in the output, and adjust the
input converter’s standalone-reasoning skip accordingly.
In `@tests/e2e/clis/clis_test.go`:
- Around line 428-433: Add a reverse-order regex branch to rateLimitSignalRE in
tests/e2e/clis/clis_test.go:428-433 that recognizes standard API-error wording
preceding a bare “rate limit,” including “API Error: rate limit.” Add “API
Error: rate limit” to the genuine-throttling cases in
tests/e2e/clis/assertion_output_test.go:208-216 so rateLimitDelay treats it as
retryable.
- Around line 797-800: Update the WalkDir callback in clearReportsDir to return
traversal errors instead of treating them as successful skips. Preserve the
existing directory-skip behavior, but when err is non-nil return it so
maybeClearReportsDir can report cleanup failures.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5bff0660-6bc5-48b1-9418-9d596aa8b20a
📒 Files selected for processing (19)
.github/workflows/release-pipeline.yml.github/workflows/scripts/test-cli-harness.shcore/providers/anthropic/reasoningreplay_test.gocore/providers/anthropic/responses.gocore/providers/bedrock/reasoning_replay_test.gocore/providers/bedrock/reasoningreplayaudit_test.gocore/providers/bedrock/responses.gocore/providers/bedrock/types.gocore/providers/bedrock/utils.gocore/providers/cohere/reasoningreplay_test.gocore/providers/cohere/responses.gocore/providers/gemini/reasoningreplay_test.gocore/providers/gemini/responses.gotests/e2e/clis/assertion_output_test.gotests/e2e/clis/clis_test.gotests/e2e/clis/errordetect_test.gotests/e2e/clis/matrix_test.gotests/e2e/clis/reasoningreplay_test.gotests/e2e/clis/scenarios_test.go
akshaydeo
force-pushed
the
08-08-encrypted_content_bug_fix
branch
from
August 8, 2026 23:20
bad5173 to
c9e09be
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/scripts/test-provider-harness.sh:
- Around line 191-201: Update the rendering failure branch around
harness-viewer.mjs to create a fallback provider-harness/index.html after the
command fails. Write minimal HTML containing links to harness-failures.md and
harness-token-parity.md, while preserving the existing warning and artifact
behavior.
In `@tests/e2e/api/runners/harness-viewer.mjs`:
- Around line 446-452: Update the static-report path around the args.static
branch to build a sanitized public-report model from items before serialization.
Redact or remove sensitive request and response headers, query parameters,
request bodies, and response bodies, including Authorization and API-key
credentials. Serialize only the sanitized model into the static HTML, while
preserving raw items for the local live viewer.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 008f2fa4-f88f-467a-8b62-0da682ddd5d6
📒 Files selected for processing (4)
.github/workflows/release-pipeline.yml.github/workflows/scripts/push-mintlify-changelog.sh.github/workflows/scripts/test-provider-harness.shtests/e2e/api/runners/harness-viewer.mjs
akshaydeo
force-pushed
the
08-08-encrypted_content_bug_fix
branch
from
August 9, 2026 00:20
c9e09be to
ea33ec2
Compare
akshaydeo
force-pushed
the
08-07-docs_and_test_case_improvements
branch
from
August 9, 2026 00:20
c44b750 to
b762b00
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
.github/workflows/release-pipeline.yml (3)
2744-2758: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGuard CLI label extraction when the resolver is skipped.
When
resolve-cli-versionsis skipped, itsmatrixoutput is empty.push-mintlify-changelogcan still run for framework-only, plugin-only, orskip-testsreleases.fromJSONthen fails while GitHub evaluatesCLI_HARNESS_LABELS.Proposed fix
- CLI_HARNESS_LABELS: ${{ join(fromJSON(needs.resolve-cli-versions.outputs.matrix).*.label, ' ') }} + CLI_HARNESS_LABELS: ${{ join(fromJSON(needs.resolve-cli-versions.outputs.matrix || '[]').*.label, ' ') }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release-pipeline.yml around lines 2744 - 2758, Guard CLI_HARNESS_LABELS extraction in push-mintlify-changelog so fromJSON is evaluated only when resolve-cli-versions succeeds and its matrix output exists. When resolve-cli-versions is skipped, use the empty-label fallback while preserving normal label extraction for successful resolver runs. Ensure the job remains runnable for framework-only, plugin-only, and skip-tests releases.
284-284: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd
api.deepseek.com:443to both hardened-job allowlists.When
DEEPSEEK_API_KEYis set, both jobs run DeepSeek integration cases. The provider useshttps://api.deepseek.com, but neithertest-corenortest-api-integrationsallows that host.egress-policy: blockwill prevent these requests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release-pipeline.yml at line 284, Add api.deepseek.com:443 to the hardened-job egress allowlists for both test-core and test-api-integrations, alongside the existing provider hosts. Keep the DEEPSEEK_API_KEY configuration unchanged and ensure both jobs permit DeepSeek integration requests when egress-policy is block.
346-349: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPass report arguments through step-level
envvariables.
transport-versionis read fromtransports/versionwithout format validation. Direct interpolation allows shell substitution before Bash parses these commands, while the steps hold R2 credentials. Validate the version and use quoted environment variables.matrix.labelis currently fixed, but pass it throughenvas well.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release-pipeline.yml around lines 346 - 349, Update the report-upload step invoking upload-test-reports-to-r2.sh to pass transport-version and matrix.label through step-level env variables. Validate transport-version’s expected format before use, then reference both values via quoted environment-variable expansions rather than directly interpolating GitHub expressions in the shell command.Source: Linters/SAST tools
tests/e2e/clis/waitdelayunix_test.go (2)
45-50: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winKill the orphan process group before returning.
cmd.Wait()at Line 50 reaps the shell beforet.Cleanupruns.syscall.Getpgid(cmd.Process.Pid)then returnsESRCH, so the backgroundsleep 30survives after each test.Capture the raw wait error, then kill
-cmd.Process.Pidimmediately.Setpgid: truemakes that PID the process-group ID while the holder still exists.Proposed fix
- t.Cleanup(func() { - if pgid, err := syscall.Getpgid(cmd.Process.Pid); err == nil && pgid > 1 { - _ = syscall.Kill(-pgid, syscall.SIGKILL) - } - }) - return cmd, cmd.Wait() + raw := cmd.Wait() + _ = syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) + return cmd, raw🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/clis/waitdelayunix_test.go` around lines 45 - 50, Update the command setup and wait flow around cmd.Wait so the process is started with Setpgid enabled, capture the raw wait error, kill the process group using -cmd.Process.Pid before returning, and then return the captured wait error. Remove the deferred t.Cleanup-based Getpgid lookup because it runs after the shell has been reaped.
1-1: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse explicit GOOS build constraints and preserve orphan cleanup.
- Replace
unixwith the supported GOOS expression, such aslinux || darwin; Go does not defineunixautomatically, so normal Linux tests select the!unixfallback.- Capture the process-group ID before
cmd.Wait(). AfterWait()reaps the shell,Getpgid(cmd.Process.Pid)fails and leavessleep 30running.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/clis/waitdelayunix_test.go` at line 1, Update the build constraint in waitdelayunix_test.go to use explicit supported GOOS values such as linux || darwin, and adjust the test’s process cleanup to capture the process-group ID before cmd.Wait(). Use the captured group ID after Wait() to terminate the orphaned sleep process.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/api/runners/lib/redact-report.mjs`:
- Around line 76-91: Update redactUrl to decode each query parameter name before
comparing it with SECRET_PARAMS, while preserving the original name in the
redacted output. Also redact URL username and password even when no query string
exists, and add regression cases covering encoded secret names and URL userinfo
credentials.
In `@tests/e2e/api/runners/lib/redact-report.test.mjs`:
- Around line 21-37: Update the item fixture to include both SECRETS[1] and
SECRETS[3] in request or response body content, alongside the existing
credential patterns, so the serialization loop exercises AWS access-key and JWT
redaction. Preserve the existing final serialized-output assertion.
---
Outside diff comments:
In @.github/workflows/release-pipeline.yml:
- Around line 2744-2758: Guard CLI_HARNESS_LABELS extraction in
push-mintlify-changelog so fromJSON is evaluated only when resolve-cli-versions
succeeds and its matrix output exists. When resolve-cli-versions is skipped, use
the empty-label fallback while preserving normal label extraction for successful
resolver runs. Ensure the job remains runnable for framework-only, plugin-only,
and skip-tests releases.
- Line 284: Add api.deepseek.com:443 to the hardened-job egress allowlists for
both test-core and test-api-integrations, alongside the existing provider hosts.
Keep the DEEPSEEK_API_KEY configuration unchanged and ensure both jobs permit
DeepSeek integration requests when egress-policy is block.
- Around line 346-349: Update the report-upload step invoking
upload-test-reports-to-r2.sh to pass transport-version and matrix.label through
step-level env variables. Validate transport-version’s expected format before
use, then reference both values via quoted environment-variable expansions
rather than directly interpolating GitHub expressions in the shell command.
In `@tests/e2e/clis/waitdelayunix_test.go`:
- Around line 45-50: Update the command setup and wait flow around cmd.Wait so
the process is started with Setpgid enabled, capture the raw wait error, kill
the process group using -cmd.Process.Pid before returning, and then return the
captured wait error. Remove the deferred t.Cleanup-based Getpgid lookup because
it runs after the shell has been reaped.
- Line 1: Update the build constraint in waitdelayunix_test.go to use explicit
supported GOOS values such as linux || darwin, and adjust the test’s process
cleanup to capture the process-group ID before cmd.Wait(). Use the captured
group ID after Wait() to terminate the orphaned sleep process.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: adb56960-3a54-4f93-a2eb-bd1a7f88d23a
📒 Files selected for processing (13)
.github/workflows/release-pipeline.yml.github/workflows/scripts/push-mintlify-changelog.sh.github/workflows/scripts/test-cli-harness.sh.github/workflows/scripts/test-provider-harness.shcore/providers/bedrock/reasoningreplayaudit_test.gotests/e2e/api/runners/harness-viewer.mjstests/e2e/api/runners/lib/redact-report.mjstests/e2e/api/runners/lib/redact-report.test.mjstests/e2e/clis/assertion_output_test.gotests/e2e/clis/clis_test.gotests/e2e/clis/reasoningreplay_test.gotests/e2e/clis/scenarios_test.gotests/e2e/clis/waitdelayunix_test.go
🚧 Files skipped from review as they are similar to previous changes (7)
- tests/e2e/clis/clis_test.go
- tests/e2e/clis/scenarios_test.go
- tests/e2e/api/runners/harness-viewer.mjs
- core/providers/bedrock/reasoningreplayaudit_test.go
- .github/workflows/scripts/test-cli-harness.sh
- tests/e2e/clis/reasoningreplay_test.go
- tests/e2e/clis/assertion_output_test.go
akshaydeo
force-pushed
the
08-08-encrypted_content_bug_fix
branch
from
August 9, 2026 01:14
ea33ec2 to
a1b3bb0
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/providers/cohere/responses.go`:
- Around line 1663-1682: The conversion in
core/providers/cohere/responses.go:1663-1682 must append
ResponsesReasoning.EncryptedContent even when content-block reasoning was
emitted: remove emittedFromContentBlocks from the early-return condition and use
it only to gate the summary loop. In
core/providers/cohere/reasoningreplay_test.go:164-207, feed the restored message
through convertBifrostReasoningToCohereThinking and add a table case combining a
reasoning content block with non-empty EncryptedContent, asserting the marker
block is preserved.
In `@tests/e2e/api/collections/provider-harness.json`:
- Around line 48655-48672: Add a marker assertion to the capture-response
parsing block after pm.response.json() in the Cohere reasoning test, verifying
the initial response output contains ENCRYPTED_REASONING. Keep the existing
replay-response assertion and capture logic unchanged.
- Around line 48769-48805: Make the capture logic in the Gemini response test
mandatory: assert that the initial output contains both a reasoning item and a
function_call with a call_id, and fail the test when either is absent instead of
silently skipping. Add the Responses API function-object tool_choice for
get_time to the replay request/input before storing
geminiReasoningReplayInput5982, ensuring the replay always exercises
thought-signature preservation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4c9730c4-bd00-405a-8c29-7791ed1896af
📒 Files selected for processing (3)
core/providers/cohere/reasoningreplay_test.gocore/providers/cohere/responses.gotests/e2e/api/collections/provider-harness.json
akshaydeo
force-pushed
the
08-07-docs_and_test_case_improvements
branch
from
August 9, 2026 02:14
b762b00 to
4424470
Compare
akshaydeo
force-pushed
the
08-08-encrypted_content_bug_fix
branch
from
August 9, 2026 02:14
a1b3bb0 to
c96dc17
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/e2e/clis/waitdelayunix_test.go (1)
242-245: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winWait for process-group removal before asserting cleanup.
SIGKILLcan leave the orphaned child as a zombie until its new parent reaps it. In that interval,syscall.Kill(-pgid, 0)can still succeed. This immediate assertion can fail intermittently.Poll for
syscall.ESRCHwith a short deadline.Proposed fix
- if err := syscall.Kill(-pgid, 0); err == nil { - t.Error("the backgrounded child survived cleanup; it will outlive the suite") - } + deadline := time.Now().Add(time.Second) + for { + err := syscall.Kill(-pgid, 0) + if errors.Is(err, syscall.ESRCH) { + break + } + if err != nil { + t.Fatalf("check process group: %v", err) + } + if time.Now().After(deadline) { + t.Error("the backgrounded child survived cleanup; it will outlive the suite") + break + } + time.Sleep(10 * time.Millisecond) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/clis/waitdelayunix_test.go` around lines 242 - 245, Replace the immediate process-group assertion after the subtest cleanup with polling that repeatedly calls syscall.Kill(-pgid, 0) until it returns syscall.ESRCH or a short deadline expires. Only report the cleanup failure after the deadline, while preserving the existing error message and successful behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release-pipeline.yml:
- Line 347: Update both R2 credentialed upload steps at
.github/workflows/release-pipeline.yml lines 347 and 636 to pass the transport
version through the TRANSPORT_VERSION environment variable and invoke
detect-all-changes.sh with the quoted variable, or otherwise validate the
version before interpolation; apply the same safe handling at both sites.
In `@core/providers/cohere/reasoningreplay_test.go`:
- Around line 216-249: Extend
TestEncryptedTokenSurvivesAlongsideContentBlockReasoning to pass the generated
blocks through convertSingleCohereMessageToBifrostMessages, then convert the
returned reasoning message back with convertBifrostReasoningToCohereThinking.
Assert on the final Cohere thinking blocks that both the visible reasoning text
and encrypted replay marker are preserved.
In `@tests/e2e/api/runners/lib/redact-report.mjs`:
- Around line 90-94: Update redactUserinfo to replace the entire matched
userinfo segment with REDACTED, without retaining the username or parsing it
separately. Add a regression test covering a username-only URL userinfo value
such as an API key and verify the resulting URL contains only the redaction.
---
Outside diff comments:
In `@tests/e2e/clis/waitdelayunix_test.go`:
- Around line 242-245: Replace the immediate process-group assertion after the
subtest cleanup with polling that repeatedly calls syscall.Kill(-pgid, 0) until
it returns syscall.ESRCH or a short deadline expires. Only report the cleanup
failure after the deadline, while preserving the existing error message and
successful behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e30bae0d-b88f-49d9-84b9-7be3fb9fbc06
📒 Files selected for processing (8)
.github/workflows/release-pipeline.yml.github/workflows/scripts/push-mintlify-changelog.shcore/providers/cohere/reasoningreplay_test.gocore/providers/cohere/responses.gotests/e2e/api/collections/provider-harness.jsontests/e2e/api/runners/lib/redact-report.mjstests/e2e/api/runners/lib/redact-report.test.mjstests/e2e/clis/waitdelayunix_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/e2e/api/collections/provider-harness.json
- .github/workflows/scripts/push-mintlify-changelog.sh
- core/providers/cohere/responses.go
akshaydeo
force-pushed
the
08-07-docs_and_test_case_improvements
branch
from
August 9, 2026 08:38
4424470 to
3d7b43b
Compare
akshaydeo
force-pushed
the
08-08-encrypted_content_bug_fix
branch
from
August 9, 2026 08:38
c96dc17 to
bb49fa1
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.github/workflows/release-pipeline.yml (4)
427-428: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDisable persisted checkout credentials in
resolve-cli-versions.This job executes a repository script. Set
persist-credentials: falsebecause it does not require Git authentication after checkout.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release-pipeline.yml around lines 427 - 428, Update the “Checkout repository” step in the resolve-cli-versions job to set persist-credentials to false, while preserving the existing actions/checkout version and other checkout settings.Source: Linters/SAST tools
576-590: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRemove provider credentials from CLI child environments.
tests/e2e/clis/runner_test.gobuilds each CLI environment fromos.Environ(). Claude Code, Codex, and OpenCode therefore inherit the Azure, AWS, andBIFROST_ENCRYPTION_KEYvalues. Keep these credentials only in the Bifrost process environment, or filter them before starting each CLI child.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release-pipeline.yml around lines 576 - 590, Update the CLI child-environment setup used by the runner around the environment construction from os.Environ() so provider credentials are not inherited by Claude Code, Codex, or OpenCode. Remove or filter AZURE_API_KEY, AZURE_ENDPOINT, AZURE_API_VERSION, AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_SESSION_TOKEN, AWS_REGION, and BIFROST_ENCRYPTION_KEY before starting each CLI child, while retaining them in the Bifrost process environment.
327-331: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winHandle incomplete provider-harness publication before emitting changelog links.
PARALLEL=1suppresses Newman’shtmlextraoutput, buttest-provider-harness.shcreates a static HTML report and a fallback page. The workflow can still omitindex.htmlbecause the copy commands ignore errors, while the uploader allows partial or skipped uploads. Requireindex.htmland a complete upload, or omit the provider-harness links when publication is incomplete.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release-pipeline.yml around lines 327 - 331, Update the provider-harness publication and changelog-link flow in the release workflow: require the generated index.html and verify the uploader completes the full upload, rather than ignoring copy errors or accepting partial uploads. If either requirement fails, skip emitting the provider-harness links.
320-320: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRemove or redact raw harness logs before artifact upload.
bifrost-dev.logcaptures unfiltered gateway output, and CLI reports retain raw transcripts pluspromptandresponsefields. Remove these files from the artifact paths or redact credentials, headers, request/response data, prompts, tool data, and reasoning before the 30-day upload.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release-pipeline.yml at line 320, Update the release pipeline’s artifact upload configuration to exclude tmp/bifrost-dev.log and any CLI report files containing raw transcripts or prompt/response fields, or ensure those files are fully redacted before upload. Preserve only sanitized diagnostics that remove credentials, headers, request/response data, prompts, tool data, and reasoning from the 30-day artifact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/api/runners/lib/redact-report.mjs`:
- Around line 97-114: Update redactUrl to process URL fragments in addition to
query parameters, splitting fragment parameters and replacing values whose
decoded names match SECRET_PARAMS while preserving non-credential fragments. Add
a regression test covering a URL with `#access_token`=... and verify the token is
redacted.
---
Outside diff comments:
In @.github/workflows/release-pipeline.yml:
- Around line 427-428: Update the “Checkout repository” step in the
resolve-cli-versions job to set persist-credentials to false, while preserving
the existing actions/checkout version and other checkout settings.
- Around line 576-590: Update the CLI child-environment setup used by the runner
around the environment construction from os.Environ() so provider credentials
are not inherited by Claude Code, Codex, or OpenCode. Remove or filter
AZURE_API_KEY, AZURE_ENDPOINT, AZURE_API_VERSION, AWS_ACCESS_KEY_ID,
AWS_SECRET_ACCESS_KEY, AWS_SESSION_TOKEN, AWS_REGION, and BIFROST_ENCRYPTION_KEY
before starting each CLI child, while retaining them in the Bifrost process
environment.
- Around line 327-331: Update the provider-harness publication and
changelog-link flow in the release workflow: require the generated index.html
and verify the uploader completes the full upload, rather than ignoring copy
errors or accepting partial uploads. If either requirement fails, skip emitting
the provider-harness links.
- Line 320: Update the release pipeline’s artifact upload configuration to
exclude tmp/bifrost-dev.log and any CLI report files containing raw transcripts
or prompt/response fields, or ensure those files are fully redacted before
upload. Preserve only sanitized diagnostics that remove credentials, headers,
request/response data, prompts, tool data, and reasoning from the 30-day
artifact.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 03b0dfa5-2a26-4093-bb96-45b1255a9813
📒 Files selected for processing (5)
.github/workflows/release-pipeline.yml.github/workflows/scripts/detect-all-changes.shcore/providers/cohere/reasoningreplay_test.gotests/e2e/api/runners/lib/redact-report.mjstests/e2e/api/runners/lib/redact-report.test.mjs
akshaydeo
force-pushed
the
08-07-docs_and_test_case_improvements
branch
from
August 9, 2026 15:14
3d7b43b to
6193b27
Compare
akshaydeo
force-pushed
the
08-08-encrypted_content_bug_fix
branch
from
August 9, 2026 15:14
bb49fa1 to
84090eb
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/api/runners/lib/redact-report.mjs`:
- Around line 134-137: Update redactBody in
tests/e2e/api/runners/lib/redact-report.mjs:134-137 to detect embedded HTTP(S)
URLs and pass each through redactUrl before applying BODY_SECRET_PATTERNS.
Update the body fixture and assertions in
tests/e2e/api/runners/lib/redact-report.test.mjs:44-50 to include an
X-Amz-Signature query credential and verify its value is absent after
serialization.
In `@tests/e2e/api/runners/lib/redact-report.test.mjs`:
- Around line 44-50: Extend the “no credential survives serialization of the
public report” test around redactItemsForPublic to include a request or response
body containing a signed URL with X-Amz-Signature set to a value outside
BODY_SECRET_PATTERNS. Assert the serialized report omits the signature value
while preserving the URL’s non-secret parameters.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 130532de-57c2-4cee-a17a-d1a5b0b6c99d
📒 Files selected for processing (3)
.github/workflows/release-pipeline.ymltests/e2e/api/runners/lib/redact-report.mjstests/e2e/api/runners/lib/redact-report.test.mjs
akshaydeo
force-pushed
the
08-08-encrypted_content_bug_fix
branch
from
August 9, 2026 15:45
84090eb to
af5d3df
Compare
akshaydeo
force-pushed
the
08-07-docs_and_test_case_improvements
branch
from
August 9, 2026 15:45
6193b27 to
3a09d4e
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/api/runners/lib/redact-report.mjs`:
- Around line 137-147: Update URL_IN_BODY and the redactBody flow to recognize
JSON slash-escaped URL separators and escaped characters, ensuring matched URLs
still pass through redactUrl and signed query parameters are redacted. In
tests/e2e/api/runners/lib/redact-report.mjs lines 137-147, modify the matcher
accordingly; in tests/e2e/api/runners/lib/redact-report.test.mjs lines 121-134,
add an https:\/\/ signed-URL fixture and assert its signature is absent.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b63b3361-62cc-4339-9be4-530fcf77b369
📒 Files selected for processing (3)
.github/workflows/scripts/detect-all-changes.shtests/e2e/api/runners/lib/redact-report.mjstests/e2e/api/runners/lib/redact-report.test.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/scripts/detect-all-changes.sh
akshaydeo
force-pushed
the
08-07-docs_and_test_case_improvements
branch
from
August 9, 2026 15:49
3a09d4e to
395b2b0
Compare
akshaydeo
force-pushed
the
08-08-encrypted_content_bug_fix
branch
2 times, most recently
from
August 9, 2026 16:01
b109226 to
706e866
Compare
akshaydeo
force-pushed
the
08-07-docs_and_test_case_improvements
branch
from
August 9, 2026 16:01
395b2b0 to
0abf7ba
Compare
akshaydeo
force-pushed
the
08-07-docs_and_test_case_improvements
branch
from
August 9, 2026 16:09
0abf7ba to
ec3ac22
Compare
akshaydeo
force-pushed
the
08-08-encrypted_content_bug_fix
branch
from
August 9, 2026 16:09
706e866 to
0a86a83
Compare
akshaydeo
force-pushed
the
08-07-docs_and_test_case_improvements
branch
from
August 9, 2026 16:15
ec3ac22 to
e9bee51
Compare
akshaydeo
force-pushed
the
08-08-encrypted_content_bug_fix
branch
from
August 9, 2026 16:15
0a86a83 to
7280280
Compare
Contributor
Author
Merge activity
|
akshaydeo
changed the base branch from
08-07-docs_and_test_case_improvements
to
graphite-base/5982
August 9, 2026 16:17
akshaydeo
dismissed
coderabbitai[bot]’s stale review
August 9, 2026 16:17
The base branch was changed.
Contributor
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
akshaydeo
force-pushed
the
08-08-encrypted_content_bug_fix
branch
from
August 9, 2026 16:18
7280280 to
7d0d712
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| }); | ||
| const [out] = redactItemsForPublic([{ idx: 0, name: "x", respBody: body }]); | ||
| assert.ok(!out.respBody.includes(signature), `signed URL leaked from a body: ${out.respBody}`); | ||
| assert.ok(out.respBody.includes("bucket.s3.amazonaws.com"), "the host must stay legible"); |
| const body = `{"url":"https:\\/\\/bucket.s3.amazonaws.com\\/report.json?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Signature=${signature}"}`; | ||
| const [out] = redactItemsForPublic([{ idx: 0, name: "x", respBody: body }]); | ||
| assert.ok(!out.respBody.includes(signature), `slash-escaped signed URL leaked: ${out.respBody}`); | ||
| assert.ok(out.respBody.includes("bucket.s3.amazonaws.com"), "the host must stay legible"); |
akshaydeo
added a commit
that referenced
this pull request
Aug 10, 2026
## Summary Fixes a series of bugs in reasoning/thinking block handling across the Anthropic, Bedrock, Cohere, and Gemini provider converters that caused replayed reasoning to be dropped, rejected, or corrupted during multi-turn conversations. Bumps the version to 1.7.8. ## Changes - Anthropic, Bedrock, and Cohere converters now fall through to `ResponsesReasoning` instead of being shadowed by an `else if`, preventing replayed reasoning from being silently dropped when `ContentBlocks` is non-nil but empty. - Bedrock no longer sends reasoning blocks with an absent `text` key — a nil pointer with `omitempty` caused Converse to reject the turn with "Member must not be null" on both the Responses and chat conversion paths. - Bedrock now attaches the replayed signature to the first reasoning summary block via `reasoningSignatureForBedrock`, rather than discarding it. - Bedrock invoke thinking blocks now always include the `thinking` key even when text is empty, preventing signature-only replay blocks from being dropped by Bifrost's decoder. - Cohere now emits encrypted reasoning alongside the summary rather than replacing it, and the `[ENCRYPTED_REASONING: ...]` marker is parsed back into `EncryptedContent` on ingress so it no longer surfaces as visible reasoning text to clients. - Gemini streaming path now base64-decodes `encrypted_content` before assigning it to `thoughtSignature`, fixing a double-encoding bug that caused Gemini to reject the signature. - Gemini now carries `thoughtSignature` from thought parts into `encrypted_content` and the reasoning block signature, and emits signature-only thought parts so clients can replay them. - Standalone Gemini reasoning messages are no longer skipped when converting Responses history to Gemini contents — thought text is resent as required, and the signature is emitted when no preceding function call consumed it. - A consumed reasoning item's thought text is now carried alongside the signature taken by a preceding Gemini function call, ensuring the thought block reaches Gemini unmodified. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go version go test ./... ``` Validate multi-turn conversations with reasoning/thinking blocks against Anthropic, Bedrock, Cohere, and Gemini providers. Confirm that: - Replayed reasoning blocks are preserved across turns. - Bedrock does not return "Member must not be null" errors on reasoning turns. - Cohere encrypted reasoning does not appear as visible text to clients. - Gemini signatures are correctly decoded and accepted on streaming responses. ## Breaking changes - [ ] Yes - [x] No ## Related issues Closes #5982, #5984 ## Security considerations None beyond ensuring that encrypted reasoning content (`encrypted_content`) is handled correctly and not exposed to clients as plaintext. ## 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
atharvamhaske
pushed a commit
to atharvamhaske/bifrost
that referenced
this pull request
Aug 13, 2026
## Summary Fixes a class of reasoning-replay defects that caused Bedrock to reject requests with `reasoningContent.reasoningText.text ... Member must not be null`. The root cause was that `BedrockReasoningContentText.Text` is `*string json:"text,omitempty"`, so a nil pointer silently drops the `text` key from the serialised request rather than sending an explicit null — and Bedrock rejects the resulting block before reading a token. The same structural bug (`else if` on a non-nil but empty `ContentBlocks` slice shadowing the encrypted-content fallback) was present across the Anthropic, Bedrock, Cohere, and Gemini converters. A new `opencode-anthropic` CLI harness suite is added to cover the Anthropic Messages wire path, which is the only one that replays reasoning verbatim and is therefore the only one that could reproduce the reported 400. ## Changes - **`core/providers/bedrock/responses.go`**: `convertBifrostReasoningToBedrockReasoning` now tracks whether content blocks actually yielded reasoning (`emittedFromContentBlocks`) rather than branching on `Content != nil`. An empty-but-non-nil `ContentBlocks` slice no longer shadows the `ResponsesReasoning` fallback. The encrypted-content branch now emits an empty-string `Text` (not nil) so the `text` key is always present on the wire. The summary branch now attaches the signature to the first block only. - **`core/providers/bedrock/utils.go`**: `convertMessage` (chat-completions path) guards `detail.Text` against nil before assigning it to `BedrockReasoningContentText.Text`, defaulting to an empty string so the key survives serialisation. - **`core/providers/bedrock/types.go`**: `BedrockInvokeMessagesContentBlock` gains a `MarshalJSON` that forces the `thinking` key to be present on thinking blocks even when the text is empty, preventing Bifrost's own re-ingest from silently dropping the block. - **`core/providers/anthropic/responses.go`**: Same `emittedFromContentBlocks` fix as Bedrock — an empty or non-reasoning `ContentBlocks` slice no longer shadows the `ResponsesReasoning` fallback. - **`core/providers/cohere/responses.go`**: Same fix, plus corrects `Summary != nil` to `len(Summary) > 0` (the nil check was always true for the empty-but-non-nil slice every construction site produces, making the encrypted-content branch permanently unreachable dead code). Summary and encrypted content are now emitted independently rather than as an either/or. - **`core/providers/gemini/responses.go`**: Extracts `thoughtSignatureFromEncryptedContent` to centralise the base64 decode. The streaming converter was assigning `[]byte(encryptedContent)` directly, producing `base64(base64(signature))` on the wire; both paths now go through the helper. The ingress converter now preserves `ThoughtSignature` from thought parts and emits a reasoning message even when the thought text is empty. - **`tests/e2e/clis/matrix_test.go`**: Adds the `opencode-anthropic` CLI entry, wired to Bifrost's `/anthropic/v1/messages` path via `@ai-sdk/anthropic`. Adds `isAnthropicFamilyModel` helper. - **`tests/e2e/clis/scenarios_test.go`**: Adds `reasoningToolReplayScenario` — a three-turn scenario where each assistant turn carries reasoning and a tool call, which is the exact shape that triggered the reported 400. The `opencode-responses` scoping is replaced by `opencode-anthropic` scoping on `isAnthropicFamilyModel`. - **`tests/e2e/clis/clis_test.go`**: Tightens `rateLimitSignalRE` to require a failure word alongside the phrase, preventing assistant prose that merely mentions "rate-limit" from triggering retries. Adds `maybeClearReportsDir` to clear stale local report artefacts once per run without affecting CI's retry loop. Registers `opencode-anthropic` in `assertionOutput`. - **`tests/e2e/clis/errordetect_test.go`**: Replaces the bare `\brate[_ -]?limit\b` error pattern with `rateLimitSignalRE`. - **`.github/workflows/release-pipeline.yml`** and **`test-cli-harness.sh`**: Add the `opencode-anthropic` suite as a fifth parallel run, raise the job timeout from 150 to 180 minutes, and include its log in the artifact upload. - New unit tests in `core/providers/anthropic/reasoningreplay_test.go`, `core/providers/bedrock/reasoning_replay_test.go`, `core/providers/bedrock/reasoningreplayaudit_test.go`, `core/providers/cohere/reasoningreplay_test.go`, and `core/providers/gemini/reasoningreplay_test.go` pin the invariants at both the struct and serialised-wire level. ## Type of change - [x] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh # Unit tests covering the fixed converters and serialisation invariants go test ./core/providers/anthropic/... ./core/providers/bedrock/... ./core/providers/cohere/... ./core/providers/gemini/... # E2E harness — runs the new reasoning-tool-replay scenario and the opencode-anthropic suite OPENCODE_ANTHROPIC_CASES="TestCLIs/opencode-anthropic/(anthropic|bedrock)/(claude-sonnet-5|global.anthropic.claude-sonnet-5)/(simple-chat|reasoning-tool-replay)" \ bash .github/workflows/scripts/test-cli-harness.sh ``` The `reasoning-tool-replay` scenario requires a model with tool-use and extended thinking enabled. The key assertion is that turn 2 answers correctly from replayed history without re-reading the file, and turn 3 succeeds after a second tool call on top of an already-replayed history. ## Breaking changes - [ ] Yes - [x] No ## Related issues The Bedrock 400 (`messages.2 ... reasoningContent.reasoningText.text ... Member must not be null`) reported from the field when replaying streamed assistant turns that contained both reasoning and tool calls. ## Security considerations None. Changes are confined to message-format conversion logic and test infrastructure. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
atharvamhaske
pushed a commit
to atharvamhaske/bifrost
that referenced
this pull request
Aug 13, 2026
## Summary Fixes a series of bugs in reasoning/thinking block handling across the Anthropic, Bedrock, Cohere, and Gemini provider converters that caused replayed reasoning to be dropped, rejected, or corrupted during multi-turn conversations. Bumps the version to 1.7.8. ## Changes - Anthropic, Bedrock, and Cohere converters now fall through to `ResponsesReasoning` instead of being shadowed by an `else if`, preventing replayed reasoning from being silently dropped when `ContentBlocks` is non-nil but empty. - Bedrock no longer sends reasoning blocks with an absent `text` key — a nil pointer with `omitempty` caused Converse to reject the turn with "Member must not be null" on both the Responses and chat conversion paths. - Bedrock now attaches the replayed signature to the first reasoning summary block via `reasoningSignatureForBedrock`, rather than discarding it. - Bedrock invoke thinking blocks now always include the `thinking` key even when text is empty, preventing signature-only replay blocks from being dropped by Bifrost's decoder. - Cohere now emits encrypted reasoning alongside the summary rather than replacing it, and the `[ENCRYPTED_REASONING: ...]` marker is parsed back into `EncryptedContent` on ingress so it no longer surfaces as visible reasoning text to clients. - Gemini streaming path now base64-decodes `encrypted_content` before assigning it to `thoughtSignature`, fixing a double-encoding bug that caused Gemini to reject the signature. - Gemini now carries `thoughtSignature` from thought parts into `encrypted_content` and the reasoning block signature, and emits signature-only thought parts so clients can replay them. - Standalone Gemini reasoning messages are no longer skipped when converting Responses history to Gemini contents — thought text is resent as required, and the signature is emitted when no preceding function call consumed it. - A consumed reasoning item's thought text is now carried alongside the signature taken by a preceding Gemini function call, ensuring the thought block reaches Gemini unmodified. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go version go test ./... ``` Validate multi-turn conversations with reasoning/thinking blocks against Anthropic, Bedrock, Cohere, and Gemini providers. Confirm that: - Replayed reasoning blocks are preserved across turns. - Bedrock does not return "Member must not be null" errors on reasoning turns. - Cohere encrypted reasoning does not appear as visible text to clients. - Gemini signatures are correctly decoded and accepted on streaming responses. ## Breaking changes - [ ] Yes - [x] No ## Related issues Closes maximhq#5982, maximhq#5984 ## Security considerations None beyond ensuring that encrypted reasoning content (`encrypted_content`) is handled correctly and not exposed to clients as plaintext. ## 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a class of reasoning-replay defects that caused Bedrock to reject requests with
reasoningContent.reasoningText.text ... Member must not be null. The root cause was thatBedrockReasoningContentText.Textis*string json:"text,omitempty", so a nil pointer silently drops thetextkey from the serialised request rather than sending an explicit null — and Bedrock rejects the resulting block before reading a token. The same structural bug (else ifon a non-nil but emptyContentBlocksslice shadowing the encrypted-content fallback) was present across the Anthropic, Bedrock, Cohere, and Gemini converters. A newopencode-anthropicCLI harness suite is added to cover the Anthropic Messages wire path, which is the only one that replays reasoning verbatim and is therefore the only one that could reproduce the reported 400.Changes
core/providers/bedrock/responses.go:convertBifrostReasoningToBedrockReasoningnow tracks whether content blocks actually yielded reasoning (emittedFromContentBlocks) rather than branching onContent != nil. An empty-but-non-nilContentBlocksslice no longer shadows theResponsesReasoningfallback. The encrypted-content branch now emits an empty-stringText(not nil) so thetextkey is always present on the wire. The summary branch now attaches the signature to the first block only.core/providers/bedrock/utils.go:convertMessage(chat-completions path) guardsdetail.Textagainst nil before assigning it toBedrockReasoningContentText.Text, defaulting to an empty string so the key survives serialisation.core/providers/bedrock/types.go:BedrockInvokeMessagesContentBlockgains aMarshalJSONthat forces thethinkingkey to be present on thinking blocks even when the text is empty, preventing Bifrost's own re-ingest from silently dropping the block.core/providers/anthropic/responses.go: SameemittedFromContentBlocksfix as Bedrock — an empty or non-reasoningContentBlocksslice no longer shadows theResponsesReasoningfallback.core/providers/cohere/responses.go: Same fix, plus correctsSummary != niltolen(Summary) > 0(the nil check was always true for the empty-but-non-nil slice every construction site produces, making the encrypted-content branch permanently unreachable dead code). Summary and encrypted content are now emitted independently rather than as an either/or.core/providers/gemini/responses.go: ExtractsthoughtSignatureFromEncryptedContentto centralise the base64 decode. The streaming converter was assigning[]byte(encryptedContent)directly, producingbase64(base64(signature))on the wire; both paths now go through the helper. The ingress converter now preservesThoughtSignaturefrom thought parts and emits a reasoning message even when the thought text is empty.tests/e2e/clis/matrix_test.go: Adds theopencode-anthropicCLI entry, wired to Bifrost's/anthropic/v1/messagespath via@ai-sdk/anthropic. AddsisAnthropicFamilyModelhelper.tests/e2e/clis/scenarios_test.go: AddsreasoningToolReplayScenario— a three-turn scenario where each assistant turn carries reasoning and a tool call, which is the exact shape that triggered the reported 400. Theopencode-responsesscoping is replaced byopencode-anthropicscoping onisAnthropicFamilyModel.tests/e2e/clis/clis_test.go: TightensrateLimitSignalREto require a failure word alongside the phrase, preventing assistant prose that merely mentions "rate-limit" from triggering retries. AddsmaybeClearReportsDirto clear stale local report artefacts once per run without affecting CI's retry loop. Registersopencode-anthropicinassertionOutput.tests/e2e/clis/errordetect_test.go: Replaces the bare\brate[_ -]?limit\berror pattern withrateLimitSignalRE..github/workflows/release-pipeline.ymlandtest-cli-harness.sh: Add theopencode-anthropicsuite as a fifth parallel run, raise the job timeout from 150 to 180 minutes, and include its log in the artifact upload.core/providers/anthropic/reasoningreplay_test.go,core/providers/bedrock/reasoning_replay_test.go,core/providers/bedrock/reasoningreplayaudit_test.go,core/providers/cohere/reasoningreplay_test.go, andcore/providers/gemini/reasoningreplay_test.gopin the invariants at both the struct and serialised-wire level.Type of change
Affected areas
How to test
The
reasoning-tool-replayscenario requires a model with tool-use and extended thinking enabled. The key assertion is that turn 2 answers correctly from replayed history without re-reading the file, and turn 3 succeeds after a second tool call on top of an already-replayed history.Breaking changes
Related issues
The Bedrock 400 (
messages.2 ... reasoningContent.reasoningText.text ... Member must not be null) reported from the field when replaying streamed assistant turns that contained both reasoning and tool calls.Security considerations
None. Changes are confined to message-format conversion logic and test infrastructure.
Checklist
docs/contributing/README.mdand followed the guidelines