harness fixes - #4580
Conversation
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughExtends the Vertex Anthropic request pre-processor to inline remote image URL sources (in addition to documents) into base64 data URIs. Adds comprehensive server tool name preservation across inbound request conversions and deep-copy operations. Prepares a release by adding changelog entries for new features and fixes, documenting the closed-issues collection workflow in the changelog-writer skill, and refreshing the E2E test harness with updated model identifiers and an Opus 4.8 feature-gating suite. ChangesVertex Remote Image URL Inlining
Server Tool Preservation and Deep Copy Enhancement
Release Changelog, Skill Documentation, and Test Harness
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
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 @.claude/skills/changelog-writer/SKILL.md:
- Around line 197-228: The GraphQL query limiting closingIssuesReferences to
first:10 will miss issues if a PR closes more than 10, and the workflow lacks
explicit deduplication when combining results from both the GraphQL per-PR
queries and the commit-body grep fallback. Increase the first:10 limit to
first:100 in the GraphQL query for closingIssuesReferences, and add an explicit
deduplication step (using sort -u on issue numbers) before rendering the final
markdown list in ascending order to eliminate any duplicate entries from the
combined sources.
🪄 Autofix (Beta)
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: bd42e42a-8b20-422c-9e4e-8f0acb110740
📒 Files selected for processing (9)
.claude/skills/changelog-writer/SKILL.mdcore/changelog.mdcore/providers/vertex/vertex.goframework/changelog.mdplugins/governance/changelog.mdplugins/logging/changelog.mdtests/config.jsontests/e2e/api/collections/provider-harness.jsontransports/changelog.md
6532bcf to
1317e16
Compare
1317e16 to
9fec659
Compare
Merge activity
|
## Summary This PR delivers several new features and bug fixes across core, framework, and plugins, including partial-usage billing for cancelled/timed-out streams, airgapped model catalog sync, MCP header forwarding, and a fix for MCP tools failing on the Bedrock/Anthropic Responses path. It also migrates the Vertex provider's URL-inlining logic to cover image blocks in addition to documents, retires the Fable 5 / Mythos test suite in favour of Claude Opus 4.8, and extends the changelog-writer skill to collect and render closed GitHub issues. ## Changes - **Failed-request billing** — Core now emits `BilledUsage` on stream cancellation and timeout errors so tokens already processed by the provider are accounted for; framework and governance/logging plugins propagate this cost through to failed and cancelled requests. - **Airgapped local sync** — Model catalog pricing and model parameters can now be loaded from local files, removing the requirement for outbound network access on airgapped deployments (closes #4305). - **MCP extra header forwarding** — Allowlisted per-request extra headers are forwarded to MCP tool calls, including `ping` and `list_tools` operations. - **Bedrock MCP tool stripping** — Provider-unsupported server tools (e.g. `mcp`) are now stripped on the Bedrock/Anthropic Responses path instead of failing the entire request, restoring pre-v1.5.0 behaviour (closes #3795). - **Vertex image URL inlining** — `inlineDocumentURLs` is renamed `inlineRemoteURLSources` and extended to also fetch and base64-encode remote image URL sources, since Anthropic-on-Vertex rejects `source.type: "url"` for both documents and images. - **Bedrock EventStream fix** — Bedrock stream errors are encoded as EventStream exceptions, fixing `ChecksumMismatch` / corrupted EventStream on `PostLLMHook` errors (closes #4545). - **Test harness updates** — Cross-cut Round 30 is migrated from the discontinued Fable 5 / Mythos models to Claude Opus 4.8; Bedrock image generation tests switch from `amazon.titan-image-generator-v2:0` to `amazon.nova-canvas-v1:0`; `moonshotai/kimi-k2-thinking-maas` and `minimaxai/minimax-m2-maas` are routed through Vertex Global; audio content-type responses are excluded from the shape-validation test script. - **Changelog skill** — The changelog-writer SKILL.md now includes instructions for collecting every closed GitHub issue via GraphQL and commit-body grep, confirming each issue's state, and rendering them as a `## 🐙 Closed GitHub Issues` section in `transports/changelog.md`. ## Type of change - [x] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./... ``` - Send a streaming request and cancel it mid-flight; verify `BilledUsage` is populated on the returned error and cost is recorded. - Send a chat completion to a Vertex/Claude model with a message containing a remote image URL; confirm the request succeeds and the image is inlined as a base64 data URI. - Send a request to a Bedrock/Anthropic Responses endpoint with an MCP tool in the tool list; confirm the request succeeds rather than returning an error. - On an airgapped host, place model catalog files locally and confirm pricing/parameters are loaded without outbound calls. ## Breaking changes - [ ] Yes - [x] No ## Related issues Closes #3357 Closes #3795 Closes #4305 Closes #4545 Closes #4556 Related: #2887 ## Security considerations Extra headers forwarded to MCP tool calls are restricted to an explicit allowlist; no new secrets or PII surfaces are introduced. ## 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
## Summary This PR delivers several new features and bug fixes across core, framework, and plugins, including partial-usage billing for cancelled/timed-out streams, airgapped model catalog sync, MCP header forwarding, and a fix for MCP tools failing on the Bedrock/Anthropic Responses path. It also migrates the Vertex provider's URL-inlining logic to cover image blocks in addition to documents, retires the Fable 5 / Mythos test suite in favour of Claude Opus 4.8, and extends the changelog-writer skill to collect and render closed GitHub issues. ## Changes - **Failed-request billing** — Core now emits `BilledUsage` on stream cancellation and timeout errors so tokens already processed by the provider are accounted for; framework and governance/logging plugins propagate this cost through to failed and cancelled requests. - **Airgapped local sync** — Model catalog pricing and model parameters can now be loaded from local files, removing the requirement for outbound network access on airgapped deployments (closes maximhq#4305). - **MCP extra header forwarding** — Allowlisted per-request extra headers are forwarded to MCP tool calls, including `ping` and `list_tools` operations. - **Bedrock MCP tool stripping** — Provider-unsupported server tools (e.g. `mcp`) are now stripped on the Bedrock/Anthropic Responses path instead of failing the entire request, restoring pre-v1.5.0 behaviour (closes maximhq#3795). - **Vertex image URL inlining** — `inlineDocumentURLs` is renamed `inlineRemoteURLSources` and extended to also fetch and base64-encode remote image URL sources, since Anthropic-on-Vertex rejects `source.type: "url"` for both documents and images. - **Bedrock EventStream fix** — Bedrock stream errors are encoded as EventStream exceptions, fixing `ChecksumMismatch` / corrupted EventStream on `PostLLMHook` errors (closes maximhq#4545). - **Test harness updates** — Cross-cut Round 30 is migrated from the discontinued Fable 5 / Mythos models to Claude Opus 4.8; Bedrock image generation tests switch from `amazon.titan-image-generator-v2:0` to `amazon.nova-canvas-v1:0`; `moonshotai/kimi-k2-thinking-maas` and `minimaxai/minimax-m2-maas` are routed through Vertex Global; audio content-type responses are excluded from the shape-validation test script. - **Changelog skill** — The changelog-writer SKILL.md now includes instructions for collecting every closed GitHub issue via GraphQL and commit-body grep, confirming each issue's state, and rendering them as a `## 🐙 Closed GitHub Issues` section in `transports/changelog.md`. ## Type of change - [x] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./... ``` - Send a streaming request and cancel it mid-flight; verify `BilledUsage` is populated on the returned error and cost is recorded. - Send a chat completion to a Vertex/Claude model with a message containing a remote image URL; confirm the request succeeds and the image is inlined as a base64 data URI. - Send a request to a Bedrock/Anthropic Responses endpoint with an MCP tool in the tool list; confirm the request succeeds rather than returning an error. - On an airgapped host, place model catalog files locally and confirm pricing/parameters are loaded without outbound calls. ## Breaking changes - [ ] Yes - [x] No ## Related issues Closes maximhq#3357 Closes maximhq#3795 Closes maximhq#4305 Closes maximhq#4545 Closes maximhq#4556 Related: maximhq#2887 ## Security considerations Extra headers forwarded to MCP tool calls are restricted to an explicit allowlist; no new secrets or PII surfaces are introduced. ## 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

Summary
This PR delivers several new features and bug fixes across core, framework, and plugins, including partial-usage billing for cancelled/timed-out streams, airgapped model catalog sync, MCP header forwarding, and a fix for MCP tools failing on the Bedrock/Anthropic Responses path. It also migrates the Vertex provider's URL-inlining logic to cover image blocks in addition to documents, retires the Fable 5 / Mythos test suite in favour of Claude Opus 4.8, and extends the changelog-writer skill to collect and render closed GitHub issues.
Changes
BilledUsageon stream cancellation and timeout errors so tokens already processed by the provider are accounted for; framework and governance/logging plugins propagate this cost through to failed and cancelled requests.pingandlist_toolsoperations.mcp) are now stripped on the Bedrock/Anthropic Responses path instead of failing the entire request, restoring pre-v1.5.0 behaviour (closes [Bug Report] #3795).inlineDocumentURLsis renamedinlineRemoteURLSourcesand extended to also fetch and base64-encode remote image URL sources, since Anthropic-on-Vertex rejectssource.type: "url"for both documents and images.ChecksumMismatch/ corrupted EventStream onPostLLMHookerrors (closes [Bug]: BifrostError returned from PostLLMHook during Bedrock streaming causes ChecksumMismatch / corrupted AWS EventStream #4545).amazon.titan-image-generator-v2:0toamazon.nova-canvas-v1:0;moonshotai/kimi-k2-thinking-maasandminimaxai/minimax-m2-maasare routed through Vertex Global; audio content-type responses are excluded from the shape-validation test script.## 🐙 Closed GitHub Issuessection intransports/changelog.md.Type of change
Affected areas
How to test
go test ./...BilledUsageis populated on the returned error and cost is recorded.Breaking changes
Related issues
Closes #3357
Closes #3795
Closes #4305
Closes #4545
Closes #4556
Related: #2887
Security considerations
Extra headers forwarded to MCP tool calls are restricted to an explicit allowlist; no new secrets or PII surfaces are introduced.
Checklist
docs/contributing/README.mdand followed the guidelines