fix: harden Bedrock invoke/stream request conversion and handlers

- Add optional logger to bedrockToCompletionRequest for warnings
- Use deterministic tool_use_${index} fallback IDs for missing tool_use
  block IDs instead of random toolu_* values
- Warn on unsupported content block types (image, document) that get
  dropped during conversion
- Warn on unexpected message roles
- Change textContent || null to ?? null to preserve empty strings
- Set completionReq.stream = true in handleBedrockStream
- Add webSearches warning on tool-call-only responses
- Add JSDoc to bedrockUsage explaining zero-default behavior
- Update test expectations for ?? semantics and deterministic IDs
- Import shared test helpers from helpers/mock-res.ts

Author: jpr5

src/__tests__/bedrock.test.ts

@@ -5,6 +5,7 @@ import { createServer, type ServerInstance } from "../server.js";
 import { bedrockToCompletionRequest, handleBedrock, handleBedrockStream } from "../bedrock.js";
 import { Journal } from "../journal.js";
 import { Logger } from "../logger.js";
+import { createMockReq, createMockRes, createDefaults } from "./helpers/mock-res.js";
 
 // --- helpers ---
 
@@ -635,7 +636,7 @@ describe("bedrockToCompletionRequest (edge cases)", () => {
       },
       "model",
     );
-    expect(result.messages[0].tool_calls![0].id).toMatch(/^toolu_/);
+    expect(result.messages[0].tool_calls![0].id).toBe("tool_use_0");
   });
 
   it("handles assistant tool_use block with missing name (falls back to '')", () => {
@@ -750,8 +751,8 @@ describe("bedrockToCompletionRequest (edge cases)", () => {
       },
       "model",
     );
-    // Empty array → no tool_use blocks, textContent is "" → null
-    expect(result.messages[0]).toEqual({ role: "assistant", content: null });
+    // Empty array → no tool_use blocks, textContent is "" → preserved as "" (not coerced to null via ??)
+    expect(result.messages[0]).toEqual({ role: "assistant", content: "" });
   });
 
   it("handles user message with content blocks but no tool_results (text extraction)", () => {
@@ -996,62 +997,6 @@ describe("POST /model/{modelId}/invoke (tool call with empty arguments)", () =>
 // Direct handler tests for req.method/req.url fallback branches
 // ---------------------------------------------------------------------------
 
-function createMockReq(overrides: Partial<http.IncomingMessage> = {}): http.IncomingMessage {
-  return {
-    method: undefined,
-    url: undefined,
-    headers: {},
-    ...overrides,
-  } as unknown as http.IncomingMessage;
-}
-
-function createMockRes(): http.ServerResponse & {
-  _written: string;
-  _status: number;
-  _headers: Record<string, string>;
-} {
-  const res = {
-    _written: "",
-    _status: 0,
-    _headers: {} as Record<string, string>,
-    writableEnded: false,
-    statusCode: 0,
-    writeHead(status: number, headers?: Record<string, string>) {
-      res._status = status;
-      res.statusCode = status;
-      if (headers) Object.assign(res._headers, headers);
-    },
-    setHeader(name: string, value: string) {
-      res._headers[name] = value;
-    },
-    write(data: string) {
-      res._written += data;
-      return true;
-    },
-    end(data?: string) {
-      if (data) res._written += data;
-      res.writableEnded = true;
-    },
-    destroy() {
-      res.writableEnded = true;
-    },
-  };
-  return res as unknown as http.ServerResponse & {
-    _written: string;
-    _status: number;
-    _headers: Record<string, string>;
-  };
-}
-
-function createDefaults(overrides: Partial<HandlerDefaults> = {}): HandlerDefaults {
-  return {
-    latency: 0,
-    chunkSize: 100,
-    logger: new Logger("silent"),
-    ...overrides,
-  };
-}
-
 describe("handleBedrock (direct handler call, method/url fallbacks)", () => {
   it("uses fallback values when req.method and req.url are undefined", async () => {
     const fixture: Fixture = {

src/bedrock.ts

@@ -95,6 +95,15 @@ function bedrockStopReason(
   return overrideFinishReason;
 }
 
+/**
+ * Build a Bedrock-style usage object from optional overrides.
+ *
+ * When no overrides are provided (the common case for mock fixtures),
+ * returns all-zero token counts. This is intentional — aimock does not
+ * attempt to estimate token usage from fixture content. Callers that
+ * need realistic usage numbers should set `usage` in their fixture's
+ * response overrides.
+ */
 function bedrockUsage(overrides?: ResponseOverrides): {
   input_tokens: number;
   output_tokens: number;
@@ -119,6 +128,7 @@ function extractTextContent(content: string | BedrockContentBlock[]): string {
 export function bedrockToCompletionRequest(
   req: BedrockRequest,
   modelId: string,
+  logger?: Logger,
 ): ChatCompletionRequest {
   const messages: ChatMessage[] = [];
 
@@ -140,6 +150,17 @@ export function bedrockToCompletionRequest(
     if (msg.role === "user") {
       // Check for tool_result blocks
       if (typeof msg.content !== "string" && Array.isArray(msg.content)) {
+        // Warn about non-text content blocks that will be dropped (image, document, etc.)
+        const unsupportedBlocks = msg.content.filter(
+          (b) => b.type !== "text" && b.type !== "tool_result",
+        );
+        if (unsupportedBlocks.length > 0 && logger) {
+          const types = [...new Set(unsupportedBlocks.map((b) => b.type))].join(", ");
+          logger.warn(
+            `Bedrock user message contains unsupported content block types [${types}] — these will be dropped during conversion`,
+          );
+        }
+
         const toolResults = msg.content.filter((b) => b.type === "tool_result");
         const textBlocks = msg.content.filter((b) => b.type === "text");
 
@@ -183,22 +204,35 @@ export function bedrockToCompletionRequest(
         if (toolUseBlocks.length > 0) {
           messages.push({
             role: "assistant",
-            content: textContent || null,
-            tool_calls: toolUseBlocks.map((b) => ({
-              id: b.id ?? generateToolUseId(),
-              type: "function" as const,
-              function: {
-                name: b.name ?? "",
-                arguments: typeof b.input === "string" ? b.input : JSON.stringify(b.input ?? {}),
-              },
-            })),
+            content: textContent ?? null,
+            tool_calls: toolUseBlocks.map((b, index) => {
+              if (!b.id && logger) {
+                logger.warn(
+                  `Bedrock assistant tool_use block at index ${index} is missing an id — using deterministic fallback "tool_use_${index}"`,
+                );
+              }
+              return {
+                id: b.id ?? `tool_use_${index}`,
+                type: "function" as const,
+                function: {
+                  name: b.name ?? "",
+                  arguments: typeof b.input === "string" ? b.input : JSON.stringify(b.input ?? {}),
+                },
+              };
+            }),
           });
         } else {
-          messages.push({ role: "assistant", content: textContent || null });
+          messages.push({ role: "assistant", content: textContent ?? null });
         }
       } else {
         messages.push({ role: "assistant", content: null });
       }
+    } else {
+      if (logger) {
+        logger.warn(
+          `Bedrock message has unexpected role "${(msg as { role: string }).role}" — skipping`,
+        );
+      }
     }
   }
 
@@ -347,7 +381,7 @@ export async function handleBedrock(
   }
 
   // Convert to ChatCompletionRequest for fixture matching
-  const completionReq = bedrockToCompletionRequest(bedrockReq, modelId);
+  const completionReq = bedrockToCompletionRequest(bedrockReq, modelId, logger);
   completionReq._endpointType = "chat";
 
   const testId = getTestId(req);
@@ -449,7 +483,8 @@ export async function handleBedrock(
       body: completionReq,
       response: { status, fixture },
     });
-    // Anthropic-style error format (Bedrock uses Claude): { type: "error", error: { type, message } }
+    // Bedrock Claude error format: { type: "error", error: { type, message } }
+    // Uses ?? (nullish coalescing) intentionally — preserves explicit empty-string types from fixtures.
     const anthropicError = {
       type: "error",
       error: {
@@ -526,6 +561,9 @@ export async function handleBedrock(
 
   // Tool call response
   if (isToolCallResponse(response)) {
+    if ("webSearches" in response) {
+      logger.warn("webSearches in fixture response are not supported for Bedrock API — ignoring");
+    }
     const overrides = extractOverrides(response);
     journal.add({
       method: req.method ?? "POST",
@@ -938,7 +976,8 @@ export async function handleBedrockStream(
     return;
   }
 
-  const completionReq = bedrockToCompletionRequest(bedrockReq, modelId);
+  const completionReq = bedrockToCompletionRequest(bedrockReq, modelId, logger);
+  completionReq.stream = true;
   completionReq._endpointType = "chat";
 
   const testId = getTestId(req);
@@ -1042,7 +1081,8 @@ export async function handleBedrockStream(
       body: completionReq,
       response: { status, fixture },
     });
-    // Anthropic-style error format (Bedrock uses Claude): { type: "error", error: { type, message } }
+    // Bedrock Claude error format: { type: "error", error: { type, message } }
+    // Uses ?? (nullish coalescing) intentionally — preserves explicit empty-string types from fixtures.
     const anthropicError = {
       type: "error",
       error: {
@@ -1130,6 +1170,9 @@ export async function handleBedrockStream(
 
   // Tool call response — stream as Event Stream
   if (isToolCallResponse(response)) {
+    if ("webSearches" in response) {
+      logger.warn("webSearches in fixture response are not supported for Bedrock API — ignoring");
+    }
     const overrides = extractOverrides(response);
     const journalEntry = journal.add({
       method: req.method ?? "POST",