Merge #18: bedrock /btw chats fail from invalid toolConfig
Inject a stub toolConfig when the message history contains toolUse or toolResult blocks but req.Tools is empty (e.g. the /btw side-chat sends the frozen main transcript). Bedrock's Converse API otherwise rejects the request with HTTP 400. Bedrock-only; other providers unaffected. Co-authored-by: Raymond Gasper <raymondgasper@fastmail.com>
2 files · +102 -0
M packages/provider/amazon_bedrock.go +33 -0
| @@ -299,6 +299,24 @@ func bedrockModelSupportsCaching(modelID string) bool { | |||
| 299 | 299 | return strings.HasPrefix(modelID, "anthropic.claude-") | |
| 300 | 300 | } | |
| 301 | 301 | ||
| 302 | + | // bedrockMessagesHaveToolBlocks reports whether any message in msgs | |
| 303 | + | // contains a toolUse or toolResult content block. Bedrock's Converse API | |
| 304 | + | // rejects the request with HTTP 400 if those block types are present but | |
| 305 | + | // toolConfig is absent. | |
| 306 | + | func bedrockMessagesHaveToolBlocks(msgs []bedrockMessage) bool { | |
| 307 | + | for _, m := range msgs { | |
| 308 | + | for _, c := range m.Content { | |
| 309 | + | if _, ok := c["toolUse"]; ok { | |
| 310 | + | return true | |
| 311 | + | } | |
| 312 | + | if _, ok := c["toolResult"]; ok { | |
| 313 | + | return true | |
| 314 | + | } | |
| 315 | + | } | |
| 316 | + | } | |
| 317 | + | return false | |
| 318 | + | } | |
| 319 | + | ||
| 302 | 320 | func (c *bedrockClient) buildRequest(req Request) (*bedrockRequest, error) { | |
| 303 | 321 | out := &bedrockRequest{} | |
| 304 | 322 | ||
| @@ -380,6 +398,21 @@ func (c *bedrockClient) buildRequest(req Request) (*bedrockRequest, error) { | |||
| 380 | 398 | tc.Tools = append(tc.Tools, ts) | |
| 381 | 399 | } | |
| 382 | 400 | out.ToolConfig = &tc | |
| 401 | + | } else if bedrockMessagesHaveToolBlocks(out.Messages) { | |
| 402 | + | // Bedrock requires toolConfig to be present whenever the message | |
| 403 | + | // history contains toolUse or toolResult content blocks, even if | |
| 404 | + | // the current request does not intend to call any tools (e.g. the | |
| 405 | + | // /btw side-chat sends the frozen main transcript which may include | |
| 406 | + | // prior tool turns). Inject a stub tool so the API accepts the | |
| 407 | + | // request; it will never be invoked since no tool_use stop reason | |
| 408 | + | // can be triggered when the real tool list is empty. | |
| 409 | + | var stub bedrockToolSpec | |
| 410 | + | stub.ToolSpec.Name = "_placeholder" | |
| 411 | + | stub.ToolSpec.Description = "placeholder required by Bedrock when tool history is present" | |
| 412 | + | stub.ToolSpec.InputSchema.JSON = json.RawMessage(`{"type":"object","properties":{}}`) | |
| 413 | + | out.ToolConfig = &struct { | |
| 414 | + | Tools []bedrockToolSpec `json:"tools"` | |
| 415 | + | }{Tools: []bedrockToolSpec{stub}} | |
| 383 | 416 | } | |
| 384 | 417 | ||
| 385 | 418 | if caching { | |
| @@ -299,6 +299,24 @@ func bedrockModelSupportsCaching(modelID string) bool { | |||||
| 299 | return strings.HasPrefix(modelID, "anthropic.claude-") | 299 | return strings.HasPrefix(modelID, "anthropic.claude-") | ||
| 300 | } | 300 | } | ||
| 301 | 301 | ||||
| 302 | + | // bedrockMessagesHaveToolBlocks reports whether any message in msgs | |||
| 303 | + | // contains a toolUse or toolResult content block. Bedrock's Converse API | |||
| 304 | + | // rejects the request with HTTP 400 if those block types are present but | |||
| 305 | + | // toolConfig is absent. | |||
| 306 | + | func bedrockMessagesHaveToolBlocks(msgs []bedrockMessage) bool { | |||
| 307 | + | for _, m := range msgs { | |||
| 308 | + | for _, c := range m.Content { | |||
| 309 | + | if _, ok := c["toolUse"]; ok { | |||
| 310 | + | return true | |||
| 311 | + | } | |||
| 312 | + | if _, ok := c["toolResult"]; ok { | |||
| 313 | + | return true | |||
| 314 | + | } | |||
| 315 | + | } | |||
| 316 | + | } | |||
| 317 | + | return false | |||
| 318 | + | } | |||
| 319 | + | ||||
| 302 | func (c *bedrockClient) buildRequest(req Request) (*bedrockRequest, error) { | 320 | func (c *bedrockClient) buildRequest(req Request) (*bedrockRequest, error) { | ||
| 303 | out := &bedrockRequest{} | 321 | out := &bedrockRequest{} | ||
| 304 | 322 | ||||
| @@ -380,6 +398,21 @@ func (c *bedrockClient) buildRequest(req Request) (*bedrockRequest, error) { | |||||
| 380 | tc.Tools = append(tc.Tools, ts) | 398 | tc.Tools = append(tc.Tools, ts) | ||
| 381 | } | 399 | } | ||
| 382 | out.ToolConfig = &tc | 400 | out.ToolConfig = &tc | ||
| 401 | + | } else if bedrockMessagesHaveToolBlocks(out.Messages) { | |||
| 402 | + | // Bedrock requires toolConfig to be present whenever the message | |||
| 403 | + | // history contains toolUse or toolResult content blocks, even if | |||
| 404 | + | // the current request does not intend to call any tools (e.g. the | |||
| 405 | + | // /btw side-chat sends the frozen main transcript which may include | |||
| 406 | + | // prior tool turns). Inject a stub tool so the API accepts the | |||
| 407 | + | // request; it will never be invoked since no tool_use stop reason | |||
| 408 | + | // can be triggered when the real tool list is empty. | |||
| 409 | + | var stub bedrockToolSpec | |||
| 410 | + | stub.ToolSpec.Name = "_placeholder" | |||
| 411 | + | stub.ToolSpec.Description = "placeholder required by Bedrock when tool history is present" | |||
| 412 | + | stub.ToolSpec.InputSchema.JSON = json.RawMessage(`{"type":"object","properties":{}}`) | |||
| 413 | + | out.ToolConfig = &struct { | |||
| 414 | + | Tools []bedrockToolSpec `json:"tools"` | |||
| 415 | + | }{Tools: []bedrockToolSpec{stub}} | |||
| 383 | } | 416 | } | ||
| 384 | 417 | ||||
| 385 | if caching { | 418 | if caching { | ||
M packages/provider/amazon_bedrock_test.go +69 -0
| @@ -357,6 +357,75 @@ func TestBedrockBuildRequestCachingWireJSON(t *testing.T) { | |||
| 357 | 357 | } | |
| 358 | 358 | } | |
| 359 | 359 | ||
| 360 | + | // TestBedrockBuildRequestBtwToolHistory reproduces the /btw HTTP 400 bug: | |
| 361 | + | // the side-chat sends no tools but the frozen main transcript contains | |
| 362 | + | // toolUse / toolResult blocks. Bedrock rejects such a request unless | |
| 363 | + | // toolConfig is present. buildRequest must inject a stub toolConfig. | |
| 364 | + | func TestBedrockBuildRequestBtwToolHistory(t *testing.T) { | |
| 365 | + | client := &bedrockClient{region: "us-east-1"} | |
| 366 | + | req, err := client.buildRequest(Request{ | |
| 367 | + | Model: "amazon.nova-pro-v1:0", | |
| 368 | + | // No tools — simulates /btw side-chat. | |
| 369 | + | Messages: []Message{ | |
| 370 | + | // Frozen main transcript: one tool call + result already happened. | |
| 371 | + | {Role: RoleAssistant, Content: []Content{ | |
| 372 | + | ToolCallBlock{ID: "tc-1", Name: "bash", Arguments: json.RawMessage(`{"command":"ls"}`)}, | |
| 373 | + | }}, | |
| 374 | + | {Role: RoleTool, Content: []Content{ | |
| 375 | + | ToolResultBlock{CallID: "tc-1", Content: []Content{TextBlock{Text: "file.go"}}}, | |
| 376 | + | }}, | |
| 377 | + | // Side-chat question appended by btwDialog.submit. | |
| 378 | + | {Role: RoleUser, Content: []Content{TextBlock{Text: "what does that file do?"}}}, | |
| 379 | + | }, | |
| 380 | + | }) | |
| 381 | + | if err != nil { | |
| 382 | + | t.Fatal(err) | |
| 383 | + | } | |
| 384 | + | if req.ToolConfig == nil { | |
| 385 | + | t.Fatal("buildRequest should inject a stub toolConfig when history has tool blocks and req.Tools is empty, but ToolConfig is nil") | |
| 386 | + | } | |
| 387 | + | if len(req.ToolConfig.Tools) == 0 { | |
| 388 | + | t.Fatal("stub toolConfig should have at least one tool") | |
| 389 | + | } | |
| 390 | + | // The stub must have a valid name and a non-nil schema so Bedrock won't | |
| 391 | + | // reject it for a different reason. | |
| 392 | + | stub := req.ToolConfig.Tools[0] | |
| 393 | + | if stub.ToolSpec.Name == "" { | |
| 394 | + | t.Error("stub tool name must not be empty") | |
| 395 | + | } | |
| 396 | + | if stub.ToolSpec.InputSchema.JSON == nil { | |
| 397 | + | t.Error("stub tool schema must not be nil") | |
| 398 | + | } | |
| 399 | + | // Serialised wire payload must include toolConfig. | |
| 400 | + | b, err := json.Marshal(req) | |
| 401 | + | if err != nil { | |
| 402 | + | t.Fatal(err) | |
| 403 | + | } | |
| 404 | + | if !strings.Contains(string(b), `"toolConfig"`) { | |
| 405 | + | t.Errorf("serialised request missing toolConfig: %s", b) | |
| 406 | + | } | |
| 407 | + | } | |
| 408 | + | ||
| 409 | + | // TestBedrockBuildRequestNoToolHistoryNoStub ensures that when there are | |
| 410 | + | // no tool blocks in the history and no tools in the request (ordinary | |
| 411 | + | // conversational call), we do NOT inject a toolConfig at all — the extra | |
| 412 | + | // field is unnecessary and some models may behave differently with it. | |
| 413 | + | func TestBedrockBuildRequestNoToolHistoryNoStub(t *testing.T) { | |
| 414 | + | client := &bedrockClient{region: "us-east-1"} | |
| 415 | + | req, err := client.buildRequest(Request{ | |
| 416 | + | Model: "amazon.nova-pro-v1:0", | |
| 417 | + | Messages: []Message{ | |
| 418 | + | {Role: RoleUser, Content: []Content{TextBlock{Text: "hello"}}}, | |
| 419 | + | }, | |
| 420 | + | }) | |
| 421 | + | if err != nil { | |
| 422 | + | t.Fatal(err) | |
| 423 | + | } | |
| 424 | + | if req.ToolConfig != nil { | |
| 425 | + | t.Errorf("expected nil ToolConfig for plain conversation, got: %+v", req.ToolConfig) | |
| 426 | + | } | |
| 427 | + | } | |
| 428 | + | ||
| 360 | 429 | func TestBedrockBuildRequestSkipsEmptyToolMessages(t *testing.T) { | |
| 361 | 430 | client := &bedrockClient{} | |
| 362 | 431 | req, err := client.buildRequest(Request{Messages: []Message{ | |
| @@ -357,6 +357,75 @@ func TestBedrockBuildRequestCachingWireJSON(t *testing.T) { | |||||
| 357 | } | 357 | } | ||
| 358 | } | 358 | } | ||
| 359 | 359 | ||||
| 360 | + | // TestBedrockBuildRequestBtwToolHistory reproduces the /btw HTTP 400 bug: | |||
| 361 | + | // the side-chat sends no tools but the frozen main transcript contains | |||
| 362 | + | // toolUse / toolResult blocks. Bedrock rejects such a request unless | |||
| 363 | + | // toolConfig is present. buildRequest must inject a stub toolConfig. | |||
| 364 | + | func TestBedrockBuildRequestBtwToolHistory(t *testing.T) { | |||
| 365 | + | client := &bedrockClient{region: "us-east-1"} | |||
| 366 | + | req, err := client.buildRequest(Request{ | |||
| 367 | + | Model: "amazon.nova-pro-v1:0", | |||
| 368 | + | // No tools — simulates /btw side-chat. | |||
| 369 | + | Messages: []Message{ | |||
| 370 | + | // Frozen main transcript: one tool call + result already happened. | |||
| 371 | + | {Role: RoleAssistant, Content: []Content{ | |||
| 372 | + | ToolCallBlock{ID: "tc-1", Name: "bash", Arguments: json.RawMessage(`{"command":"ls"}`)}, | |||
| 373 | + | }}, | |||
| 374 | + | {Role: RoleTool, Content: []Content{ | |||
| 375 | + | ToolResultBlock{CallID: "tc-1", Content: []Content{TextBlock{Text: "file.go"}}}, | |||
| 376 | + | }}, | |||
| 377 | + | // Side-chat question appended by btwDialog.submit. | |||
| 378 | + | {Role: RoleUser, Content: []Content{TextBlock{Text: "what does that file do?"}}}, | |||
| 379 | + | }, | |||
| 380 | + | }) | |||
| 381 | + | if err != nil { | |||
| 382 | + | t.Fatal(err) | |||
| 383 | + | } | |||
| 384 | + | if req.ToolConfig == nil { | |||
| 385 | + | t.Fatal("buildRequest should inject a stub toolConfig when history has tool blocks and req.Tools is empty, but ToolConfig is nil") | |||
| 386 | + | } | |||
| 387 | + | if len(req.ToolConfig.Tools) == 0 { | |||
| 388 | + | t.Fatal("stub toolConfig should have at least one tool") | |||
| 389 | + | } | |||
| 390 | + | // The stub must have a valid name and a non-nil schema so Bedrock won't | |||
| 391 | + | // reject it for a different reason. | |||
| 392 | + | stub := req.ToolConfig.Tools[0] | |||
| 393 | + | if stub.ToolSpec.Name == "" { | |||
| 394 | + | t.Error("stub tool name must not be empty") | |||
| 395 | + | } | |||
| 396 | + | if stub.ToolSpec.InputSchema.JSON == nil { | |||
| 397 | + | t.Error("stub tool schema must not be nil") | |||
| 398 | + | } | |||
| 399 | + | // Serialised wire payload must include toolConfig. | |||
| 400 | + | b, err := json.Marshal(req) | |||
| 401 | + | if err != nil { | |||
| 402 | + | t.Fatal(err) | |||
| 403 | + | } | |||
| 404 | + | if !strings.Contains(string(b), `"toolConfig"`) { | |||
| 405 | + | t.Errorf("serialised request missing toolConfig: %s", b) | |||
| 406 | + | } | |||
| 407 | + | } | |||
| 408 | + | ||||
| 409 | + | // TestBedrockBuildRequestNoToolHistoryNoStub ensures that when there are | |||
| 410 | + | // no tool blocks in the history and no tools in the request (ordinary | |||
| 411 | + | // conversational call), we do NOT inject a toolConfig at all — the extra | |||
| 412 | + | // field is unnecessary and some models may behave differently with it. | |||
| 413 | + | func TestBedrockBuildRequestNoToolHistoryNoStub(t *testing.T) { | |||
| 414 | + | client := &bedrockClient{region: "us-east-1"} | |||
| 415 | + | req, err := client.buildRequest(Request{ | |||
| 416 | + | Model: "amazon.nova-pro-v1:0", | |||
| 417 | + | Messages: []Message{ | |||
| 418 | + | {Role: RoleUser, Content: []Content{TextBlock{Text: "hello"}}}, | |||
| 419 | + | }, | |||
| 420 | + | }) | |||
| 421 | + | if err != nil { | |||
| 422 | + | t.Fatal(err) | |||
| 423 | + | } | |||
| 424 | + | if req.ToolConfig != nil { | |||
| 425 | + | t.Errorf("expected nil ToolConfig for plain conversation, got: %+v", req.ToolConfig) | |||
| 426 | + | } | |||
| 427 | + | } | |||
| 428 | + | ||||
| 360 | func TestBedrockBuildRequestSkipsEmptyToolMessages(t *testing.T) { | 429 | func TestBedrockBuildRequestSkipsEmptyToolMessages(t *testing.T) { | ||
| 361 | client := &bedrockClient{} | 430 | client := &bedrockClient{} | ||
| 362 | req, err := client.buildRequest(Request{Messages: []Message{ | 431 | req, err := client.buildRequest(Request{Messages: []Message{ | ||