Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions strix/core/execution.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,15 @@ async def _run_cycle( # noqa: PLR0912, PLR0915
while True:
try:
await coordinator.mark_running(agent_id)

# --- PRODUCTION FIX FOR AWS BEDROCK TOOL_CHOICE COMPLIANCE ---
if hasattr(run_config, "extra_body") and isinstance(run_config.extra_body, dict):
if "tool_choice" in run_config.extra_body:
run_config.extra_body["tool_choice"] = {"type": "auto"}
elif hasattr(run_config, "tool_choice") and (run_config.tool_choice == "auto" or isinstance(run_config.tool_choice, str)):
run_config.tool_choice = {"type": "auto"}
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Comment on lines +357 to +358

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Tool choice overwritten This branch still rewrites every string run_config.tool_choice, not only the Bedrock-incompatible "auto" value. When a caller configures "required" or "none", the shared RunConfig is mutated to {"type": "auto"} before Runner.run_streamed(), so later cycles and child agents can run with automatic tool selection instead of the configured policy.

Prompt To Fix With AI
This is a comment left during a code review.
Path: strix/core/execution.py
Line: 357-358

Comment:
**Tool choice overwritten** This branch still rewrites every string `run_config.tool_choice`, not only the Bedrock-incompatible `"auto"` value. When a caller configures `"required"` or `"none"`, the shared `RunConfig` is mutated to `{"type": "auto"}` before `Runner.run_streamed()`, so later cycles and child agents can run with automatic tool selection instead of the configured policy.

How can I resolve this? If you propose a fix, please make it concise.

# -------------------------------------------------------------

stream = Runner.run_streamed(
agent,
input=input_data,
Expand Down
41 changes: 39 additions & 2 deletions strix/runtime/backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
from __future__ import annotations

import logging
import os
import sys
from collections.abc import Awaitable, Callable
from typing import TYPE_CHECKING, Any

Expand Down Expand Up @@ -47,11 +49,46 @@ async def _docker_backend(

from strix.runtime.docker_client import StrixDockerSandboxClient

# Handle Issue #671 Edge Cases
concurrency_limits = None
default_limit = 1 if sys.platform == "win32" else None
limit_val = os.environ.get("STRIX_DOCKER_CONCURRENCY", default_limit)

if limit_val is not None:
try:
# Prevents <= 0 limits breaking the SDK validation
limit = max(1, int(limit_val))

# Try primary import path, fallback if SDK changes
try:
from agents.sandbox.artifacts import SandboxConcurrencyLimits
except ImportError:
from agents.sandbox import SandboxConcurrencyLimits
Comment on lines +63 to +66

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Wrong import path The concurrency limit type is still loaded from SDK surfaces that do not export it in the pinned API. On the default Windows path, both import attempts can fail before the Docker backend starts. If a Windows user sets STRIX_DOCKER_CONCURRENCY to another value, the same import failure is swallowed and the backend continues with concurrency_limits=None, so workspace materialization can still run concurrently.

Prompt To Fix With AI
This is a comment left during a code review.
Path: strix/runtime/backends.py
Line: 63-66

Comment:
**Wrong import path** The concurrency limit type is still loaded from SDK surfaces that do not export it in the pinned API. On the default Windows path, both import attempts can fail before the Docker backend starts. If a Windows user sets `STRIX_DOCKER_CONCURRENCY` to another value, the same import failure is swallowed and the backend continues with `concurrency_limits=None`, so workspace materialization can still run concurrently.

How can I resolve this? If you propose a fix, please make it concise.


concurrency_limits = SandboxConcurrencyLimits(
manifest_entries=limit,
local_dir_files=limit
)
logger.debug(f"Applied SandboxConcurrencyLimits: {limit}")
except (ImportError, ValueError) as e:
if sys.platform == "win32" and limit_val == 1:
# Fail loudly on Windows to avoid silent race condition
raise RuntimeError("Failed to import SandboxConcurrencyLimits required for Windows execution.") from e
logger.warning("Invalid STRIX_DOCKER_CONCURRENCY or import failed. Proceeding with defaults.")

client = StrixDockerSandboxClient(docker.from_env())
client.strix_bind_mounts = bind_mounts or []
options = DockerSandboxClientOptions(image=image, exposed_ports=exposed_ports)

# Create the session (without concurrency_limits keyword)
session = await client.create(options=options, manifest=manifest)
await session.start()

# Pass concurrency_limits where the session is actually started/materialized
start_kwargs = {}
if concurrency_limits is not None:
start_kwargs["concurrency_limits"] = concurrency_limits

await session.start(**start_kwargs)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Start rejects kwargs Passing concurrency_limits to session.start() still targets the wrong SDK call. The Docker sandbox session start() method accepts no keyword arguments, so any path that successfully builds start_kwargs fails backend startup with an unexpected-keyword error before workspace materialization can run.

Prompt To Fix With AI
This is a comment left during a code review.
Path: strix/runtime/backends.py
Line: 91

Comment:
**Start rejects kwargs** Passing `concurrency_limits` to `session.start()` still targets the wrong SDK call. The Docker sandbox session `start()` method accepts no keyword arguments, so any path that successfully builds `start_kwargs` fails backend startup with an unexpected-keyword error before workspace materialization can run.

How can I resolve this? If you propose a fix, please make it concise.

return client, session


Expand Down Expand Up @@ -90,4 +127,4 @@ def register_backend(name: str, backend: SandboxBackend) -> None:


def supported_backends() -> list[str]:
return sorted(_BACKENDS)
return sorted(_BACKENDS)