-
Notifications
You must be signed in to change notification settings - Fork 0
feat(tools): wire per-category sandbox backend selection #534
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
58a6d77
64600a7
f6fe155
c3d335e
6b09681
5aecbfd
a31ff5b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| import aiodocker | ||
| import aiodocker.containers | ||
|
|
||
| from synthorg.core.types import NotBlankStr | ||
| from synthorg.observability import get_logger | ||
| from synthorg.observability.events.docker import ( | ||
| DOCKER_CLEANUP, | ||
|
|
@@ -35,8 +36,6 @@ | |
| if TYPE_CHECKING: | ||
| from collections.abc import Mapping | ||
|
|
||
| from synthorg.core.types import NotBlankStr | ||
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
| _DEFAULT_CONFIG = DockerSandboxConfig() | ||
|
|
@@ -645,4 +644,4 @@ async def health_check(self) -> bool: | |
|
|
||
| def get_backend_type(self) -> NotBlankStr: | ||
| """Return ``'docker'``.""" | ||
| return "docker" | ||
| return NotBlankStr("docker") | ||
|
Comment on lines
645
to
+647
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== NotBlankStr definition =="
rg -n -C2 '^\s*NotBlankStr\s*=' src/synthorg/core/types.py
echo
echo "== get_backend_type implementations =="
rg -n -C2 '\bdef get_backend_type\s*\(' src/synthorg/tools/sandbox
echo
echo "== Constructor-style NotBlankStr usage =="
rg -n '\bNotBlankStr\s*\(' src/synthorgRepository: Aureliolo/synthorg Length of output: 9129 Return a plain string instead of calling
def get_backend_type(self) -> NotBlankStr:
"""Return ``'docker'``."""
return "docker" # type: ignore[return-value]Or use from typing import cast
return cast(NotBlankStr, "docker")🤖 Prompt for AI Agents |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid instantiating unrelated backends in the VERSION_CONTROL-only path.
When
sandbox_backends is None, Line 147 builds all configured backends (default + every override), but this code path only consumesToolCategory.VERSION_CONTROL. That can eagerly construct unused backends (notably Docker), adding avoidable startup failures/dependencies for git-only tool builds.♻️ Proposed fix
def _resolve_vc_sandbox( *, config: RootConfig, sandbox_backends: Mapping[str, SandboxBackend] | None, workspace: Path, ) -> SandboxBackend: @@ - if sandbox_backends is None: - sandbox_backends = build_sandbox_backends( - config=config.sandboxing, - workspace=workspace, - ) + if sandbox_backends is None: + vc_backend_name = config.sandboxing.backend_for_category( + ToolCategory.VERSION_CONTROL.value, + ) + vc_only_config = config.sandboxing.model_copy( + update={ + "default_backend": vc_backend_name, + "overrides": {}, + }, + ) + sandbox_backends = build_sandbox_backends( + config=vc_only_config, + workspace=workspace, + )As per coding guidelines, "If implementation deviates from the design spec, alert the user and explain why — user decides whether to proceed or update the spec."
🤖 Prompt for AI Agents