mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(grok): stop opencode subagent-stream hang at the config layer
The Grok pr_reviewer wedged in_progress forever: opencode's default agent ran with the subagent `task` tool enabled, spawned an Explore subagent on grok-build-0.1 whose model call opened an SSE stream that went idle, and the run hung with no timeout. - Hard-disable opencode's subagent `task` tool in the generated opencode.json. No RoboCo role uses opencode-internal subagents — work flows through the gateway verbs — so removing the tool kills the hang trigger outright. - Set provider.xai.options.timeout + chunkTimeout (operator-tunable via ROBOCO_GROK_REQUEST_TIMEOUT_MS / ROBOCO_GROK_CHUNK_TIMEOUT_MS) as the defence-in-depth backstop; chunkTimeout aborts an idle stream. - Bundle the permission + timeout + subagent knobs into an OpencodeGuards dataclass (keeps the builder under the arg-count gate). - Drop the dead ROBOCO_AGENT_TOOLS spawn env (it had no consumer); opencode tool restriction lives in the rendered config now.
This commit is contained in:
@@ -64,10 +64,6 @@ _DEFAULT_XAI_BASE_URL = "https://api.x.ai/v1"
|
|||||||
_MCP_CONFIG_IN_CONTAINER = "/app/mcp-config.json"
|
_MCP_CONFIG_IN_CONTAINER = "/app/mcp-config.json"
|
||||||
_SYSTEM_PROMPT_IN_CONTAINER = "/app/system-prompt.md"
|
_SYSTEM_PROMPT_IN_CONTAINER = "/app/system-prompt.md"
|
||||||
|
|
||||||
# Mirrors the Claude Code tool set. The grok image entrypoint applies this to
|
|
||||||
# the OpenAI-protocol CLI via the recognised `--tools` flag (not --allowed-tools).
|
|
||||||
_DEFAULT_TOOLS = "Read,Write,Edit,Bash,Grep,Glob,TodoWrite"
|
|
||||||
|
|
||||||
# Reasoning effort by role. grok-build-0.1 reasons heavily by default, and
|
# Reasoning effort by role. grok-build-0.1 reasons heavily by default, and
|
||||||
# reasoning bills at the output rate — it dominates cost (a live "say ok" call
|
# reasoning bills at the output rate — it dominates cost (a live "say ok" call
|
||||||
# emitted ~300 reasoning tokens). Code-quality roles (developer, qa, pr_reviewer)
|
# emitted ~300 reasoning tokens). Code-quality roles (developer, qa, pr_reviewer)
|
||||||
@@ -215,8 +211,6 @@ class GrokProvider(AgentProvider):
|
|||||||
"-e",
|
"-e",
|
||||||
f"ROBOCO_SYSTEM_PROMPT={_SYSTEM_PROMPT_IN_CONTAINER}",
|
f"ROBOCO_SYSTEM_PROMPT={_SYSTEM_PROMPT_IN_CONTAINER}",
|
||||||
"-e",
|
"-e",
|
||||||
f"ROBOCO_AGENT_TOOLS={_DEFAULT_TOOLS}",
|
|
||||||
"-e",
|
|
||||||
f"ROBOCO_INITIAL_PROMPT={initial_prompt or ''}",
|
f"ROBOCO_INITIAL_PROMPT={initial_prompt or ''}",
|
||||||
]
|
]
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -7,12 +7,18 @@ sets (``OPENAI_*`` + ``ROBOCO_*``) plus the mounted Claude Code
|
|||||||
as importable Python (not a shell heredoc) makes the translation unit-testable.
|
as importable Python (not a shell heredoc) makes the translation unit-testable.
|
||||||
|
|
||||||
Config shape per opencode docs (https://opencode.ai/docs/config):
|
Config shape per opencode docs (https://opencode.ai/docs/config):
|
||||||
* ``provider.<id>`` — ``@ai-sdk/openai-compatible`` with ``options.baseURL`` /
|
* ``provider.<id>`` — ``@ai-sdk/openai`` (the Responses API; see ``_PROVIDER_NPM``)
|
||||||
``options.apiKey``; ``model`` selects ``<id>/<model>``.
|
with ``options.baseURL`` / ``options.apiKey`` / ``options.timeout`` /
|
||||||
|
``options.chunkTimeout``; ``model`` selects ``<id>/<model>``.
|
||||||
* ``mcp.<name>`` — ``{type:"local", command:[...], environment:{...}}``; this
|
* ``mcp.<name>`` — ``{type:"local", command:[...], environment:{...}}``; this
|
||||||
is where RoboCo's gateway servers (roboco-flow / roboco-do / ...) are wired,
|
is where RoboCo's gateway servers (roboco-flow / roboco-do / ...) are wired,
|
||||||
translated from Claude Code's ``mcpServers`` (``command`` + ``args`` + ``env``).
|
translated from Claude Code's ``mcpServers`` (``command`` + ``args`` + ``env``).
|
||||||
* ``permission.{bash,edit}`` and ``instructions`` (system prompt + briefing).
|
* ``permission.{bash,edit}`` and ``instructions`` (system prompt + briefing).
|
||||||
|
* ``tools`` — opencode's subagent ``task`` tool is hard-disabled. No RoboCo role
|
||||||
|
uses opencode-internal subagents (work is driven through the gateway verbs),
|
||||||
|
and a ``task``-spawned subagent on ``grok-build-0.1`` whose model call opens an
|
||||||
|
idle stream hangs the parent run with no recovery (observed live on a PR
|
||||||
|
review). The request/stream timeouts below are the defence-in-depth backstop.
|
||||||
|
|
||||||
KNOWN PARITY GAP (tracked for the opencode-plugin follow-up with xAI): RoboCo's
|
KNOWN PARITY GAP (tracked for the opencode-plugin follow-up with xAI): RoboCo's
|
||||||
bash-guard (PAT-scrub) and transcript-based usage/cost capture are Claude Code
|
bash-guard (PAT-scrub) and transcript-based usage/cost capture are Claude Code
|
||||||
@@ -41,6 +47,34 @@ _PROVIDER_NPM = "@ai-sdk/openai"
|
|||||||
# secret-scrub ports the bash-guard deny rules to opencode's tool.execute.before.
|
# secret-scrub ports the bash-guard deny rules to opencode's tool.execute.before.
|
||||||
_PLUGINS = ["/app/opencode-plugins/secret-scrub.js"]
|
_PLUGINS = ["/app/opencode-plugins/secret-scrub.js"]
|
||||||
|
|
||||||
|
# opencode's built-in subagent-spawning tool. Hard-disabled in the generated
|
||||||
|
# config (see the module docstring): a RoboCo agent never spawns opencode's own
|
||||||
|
# subagents, and one that does can wedge the parent run on an idle stream.
|
||||||
|
_SUBAGENT_TOOL = "task"
|
||||||
|
|
||||||
|
# Request / stream timeouts (ms) written into ``provider.xai.options``. ``timeout``
|
||||||
|
# bounds a single model call; ``chunkTimeout`` aborts a stream that goes idle for
|
||||||
|
# this long (no chunk arrives) — the backstop for the idle-SSE hang. Both are
|
||||||
|
# operator-tunable via env (see ``main``).
|
||||||
|
_DEFAULT_REQUEST_TIMEOUT_MS = 300_000
|
||||||
|
_DEFAULT_CHUNK_TIMEOUT_MS = 120_000
|
||||||
|
|
||||||
|
|
||||||
|
def _env_int(name: str, default: int) -> int:
|
||||||
|
"""Read a positive int from env ``name``; fall back to ``default``.
|
||||||
|
|
||||||
|
A missing, blank, non-integer, or non-positive value yields ``default`` so a
|
||||||
|
typo in an operator override can never disable the timeout entirely.
|
||||||
|
"""
|
||||||
|
raw = os.environ.get(name, "").strip()
|
||||||
|
if not raw:
|
||||||
|
return default
|
||||||
|
try:
|
||||||
|
value = int(raw)
|
||||||
|
except ValueError:
|
||||||
|
return default
|
||||||
|
return value if value > 0 else default
|
||||||
|
|
||||||
|
|
||||||
@dataclass(frozen=True)
|
@dataclass(frozen=True)
|
||||||
class XaiTarget:
|
class XaiTarget:
|
||||||
@@ -51,6 +85,22 @@ class XaiTarget:
|
|||||||
model: str
|
model: str
|
||||||
|
|
||||||
|
|
||||||
|
@dataclass(frozen=True)
|
||||||
|
class OpencodeGuards:
|
||||||
|
"""Tunable runtime guards baked into a Grok ``opencode.json``.
|
||||||
|
|
||||||
|
``bash``/``edit`` gate the command/file tools; the timeouts bound a single
|
||||||
|
model call and abort an idle stream; ``disable_subagents`` removes the
|
||||||
|
subagent ``task`` tool entirely.
|
||||||
|
"""
|
||||||
|
|
||||||
|
bash_permission: str = "allow"
|
||||||
|
edit_permission: str = "allow"
|
||||||
|
request_timeout_ms: int = _DEFAULT_REQUEST_TIMEOUT_MS
|
||||||
|
chunk_timeout_ms: int = _DEFAULT_CHUNK_TIMEOUT_MS
|
||||||
|
disable_subagents: bool = True
|
||||||
|
|
||||||
|
|
||||||
def translate_mcp_servers(mcp_config: dict[str, Any]) -> dict[str, Any]:
|
def translate_mcp_servers(mcp_config: dict[str, Any]) -> dict[str, Any]:
|
||||||
"""Translate Claude Code ``mcpServers`` into opencode's ``mcp`` block.
|
"""Translate Claude Code ``mcpServers`` into opencode's ``mcp`` block.
|
||||||
|
|
||||||
@@ -81,27 +131,39 @@ def build_opencode_config(
|
|||||||
target: XaiTarget,
|
target: XaiTarget,
|
||||||
*,
|
*,
|
||||||
instruction_paths: list[str],
|
instruction_paths: list[str],
|
||||||
bash_permission: str = "allow",
|
guards: OpencodeGuards | None = None,
|
||||||
edit_permission: str = "allow",
|
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
"""Build the full ``opencode.json`` dict for a Grok agent."""
|
"""Build the full ``opencode.json`` dict for a Grok agent."""
|
||||||
return {
|
guards = guards or OpencodeGuards()
|
||||||
|
config: dict[str, Any] = {
|
||||||
"$schema": _OPENCODE_SCHEMA,
|
"$schema": _OPENCODE_SCHEMA,
|
||||||
"provider": {
|
"provider": {
|
||||||
_PROVIDER_ID: {
|
_PROVIDER_ID: {
|
||||||
"npm": _PROVIDER_NPM,
|
"npm": _PROVIDER_NPM,
|
||||||
"name": "xAI",
|
"name": "xAI",
|
||||||
"options": {"baseURL": target.base_url, "apiKey": target.api_key},
|
"options": {
|
||||||
|
"baseURL": target.base_url,
|
||||||
|
"apiKey": target.api_key,
|
||||||
|
"timeout": guards.request_timeout_ms,
|
||||||
|
"chunkTimeout": guards.chunk_timeout_ms,
|
||||||
|
},
|
||||||
"models": {target.model: {"name": target.model}},
|
"models": {target.model: {"name": target.model}},
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
"model": f"{_PROVIDER_ID}/{target.model}",
|
"model": f"{_PROVIDER_ID}/{target.model}",
|
||||||
"mcp": translate_mcp_servers(mcp_config),
|
"mcp": translate_mcp_servers(mcp_config),
|
||||||
"permission": {"bash": bash_permission, "edit": edit_permission},
|
"permission": {
|
||||||
|
"bash": guards.bash_permission,
|
||||||
|
"edit": guards.edit_permission,
|
||||||
|
},
|
||||||
"instructions": instruction_paths,
|
"instructions": instruction_paths,
|
||||||
# Command guard / secret-scrub (bash-guard parity). Baked into the image.
|
# Command guard / secret-scrub (bash-guard parity). Baked into the image.
|
||||||
"plugin": list(_PLUGINS),
|
"plugin": list(_PLUGINS),
|
||||||
}
|
}
|
||||||
|
if guards.disable_subagents:
|
||||||
|
# Remove the subagent tool entirely so the model can never invoke it.
|
||||||
|
config["tools"] = {_SUBAGENT_TOOL: False}
|
||||||
|
return config
|
||||||
|
|
||||||
|
|
||||||
def _load_mcp_config(path: str) -> dict[str, Any]:
|
def _load_mcp_config(path: str) -> dict[str, Any]:
|
||||||
@@ -129,7 +191,15 @@ def main() -> int:
|
|||||||
"ROBOCO_OPENCODE_CONFIG",
|
"ROBOCO_OPENCODE_CONFIG",
|
||||||
str(Path.home() / ".config" / "opencode" / "opencode.json"),
|
str(Path.home() / ".config" / "opencode" / "opencode.json"),
|
||||||
)
|
)
|
||||||
bash_perm = os.environ.get("ROBOCO_GROK_BASH_PERMISSION", "allow")
|
guards = OpencodeGuards(
|
||||||
|
bash_permission=os.environ.get("ROBOCO_GROK_BASH_PERMISSION", "allow"),
|
||||||
|
request_timeout_ms=_env_int(
|
||||||
|
"ROBOCO_GROK_REQUEST_TIMEOUT_MS", _DEFAULT_REQUEST_TIMEOUT_MS
|
||||||
|
),
|
||||||
|
chunk_timeout_ms=_env_int(
|
||||||
|
"ROBOCO_GROK_CHUNK_TIMEOUT_MS", _DEFAULT_CHUNK_TIMEOUT_MS
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
# Instructions = system prompt + the SessionStart briefing when mounted.
|
# Instructions = system prompt + the SessionStart briefing when mounted.
|
||||||
candidates = [system_prompt, "/app/briefing.md"]
|
candidates = [system_prompt, "/app/briefing.md"]
|
||||||
@@ -139,7 +209,7 @@ def main() -> int:
|
|||||||
_load_mcp_config(mcp_path),
|
_load_mcp_config(mcp_path),
|
||||||
target,
|
target,
|
||||||
instruction_paths=instructions,
|
instruction_paths=instructions,
|
||||||
bash_permission=bash_perm,
|
guards=guards,
|
||||||
)
|
)
|
||||||
out = Path(out_path)
|
out = Path(out_path)
|
||||||
out.parent.mkdir(parents=True, exist_ok=True)
|
out.parent.mkdir(parents=True, exist_ok=True)
|
||||||
|
|||||||
@@ -2,8 +2,15 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
|
from unittest.mock import patch
|
||||||
|
|
||||||
from roboco.llm.providers.opencode_config import (
|
from roboco.llm.providers.opencode_config import (
|
||||||
|
_DEFAULT_CHUNK_TIMEOUT_MS,
|
||||||
|
_DEFAULT_REQUEST_TIMEOUT_MS,
|
||||||
|
OpencodeGuards,
|
||||||
XaiTarget,
|
XaiTarget,
|
||||||
|
_env_int,
|
||||||
build_opencode_config,
|
build_opencode_config,
|
||||||
translate_mcp_servers,
|
translate_mcp_servers,
|
||||||
)
|
)
|
||||||
@@ -90,7 +97,60 @@ def test_build_opencode_config_bash_permission_is_tunable() -> None:
|
|||||||
{},
|
{},
|
||||||
_TARGET,
|
_TARGET,
|
||||||
instruction_paths=[],
|
instruction_paths=[],
|
||||||
bash_permission="deny",
|
guards=OpencodeGuards(bash_permission="deny"),
|
||||||
)
|
)
|
||||||
assert cfg["permission"]["bash"] == "deny"
|
assert cfg["permission"]["bash"] == "deny"
|
||||||
assert cfg["permission"]["edit"] == "allow"
|
assert cfg["permission"]["edit"] == "allow"
|
||||||
|
|
||||||
|
|
||||||
|
def test_build_opencode_config_disables_subagent_task_tool_by_default() -> None:
|
||||||
|
# The subagent `task` tool must be hard-disabled: a RoboCo role never uses
|
||||||
|
# opencode-internal subagents, and one spawned on grok-build-0.1 hung the run.
|
||||||
|
cfg = build_opencode_config(_MCP, _TARGET, instruction_paths=[])
|
||||||
|
assert cfg["tools"] == {"task": False}
|
||||||
|
|
||||||
|
|
||||||
|
def test_build_opencode_config_subagents_can_be_re_enabled() -> None:
|
||||||
|
cfg = build_opencode_config(
|
||||||
|
_MCP,
|
||||||
|
_TARGET,
|
||||||
|
instruction_paths=[],
|
||||||
|
guards=OpencodeGuards(disable_subagents=False),
|
||||||
|
)
|
||||||
|
assert "tools" not in cfg
|
||||||
|
|
||||||
|
|
||||||
|
def test_build_opencode_config_sets_default_timeouts() -> None:
|
||||||
|
# Both timeouts land under provider.<id>.options so opencode aborts a stalled
|
||||||
|
# request / idle stream instead of hanging the parent run forever.
|
||||||
|
opts = build_opencode_config(_MCP, _TARGET, instruction_paths=[])["provider"][
|
||||||
|
"xai"
|
||||||
|
]["options"]
|
||||||
|
assert opts["timeout"] == _DEFAULT_REQUEST_TIMEOUT_MS
|
||||||
|
assert opts["chunkTimeout"] == _DEFAULT_CHUNK_TIMEOUT_MS
|
||||||
|
|
||||||
|
|
||||||
|
def test_build_opencode_config_timeouts_are_tunable() -> None:
|
||||||
|
req_ms, chunk_ms = 111_000, 22_000
|
||||||
|
opts = build_opencode_config(
|
||||||
|
_MCP,
|
||||||
|
_TARGET,
|
||||||
|
instruction_paths=[],
|
||||||
|
guards=OpencodeGuards(request_timeout_ms=req_ms, chunk_timeout_ms=chunk_ms),
|
||||||
|
)["provider"]["xai"]["options"]
|
||||||
|
assert opts["timeout"] == req_ms
|
||||||
|
assert opts["chunkTimeout"] == chunk_ms
|
||||||
|
|
||||||
|
|
||||||
|
def test_env_int_parses_and_falls_back() -> None:
|
||||||
|
fallback = 999
|
||||||
|
parsed = 45_000
|
||||||
|
with patch.dict(os.environ, {"X_MS": str(parsed)}):
|
||||||
|
assert _env_int("X_MS", fallback) == parsed
|
||||||
|
# Missing, blank, non-integer, and non-positive all fall back to the default
|
||||||
|
# so a bad operator override can never disable the timeout entirely.
|
||||||
|
with patch.dict(os.environ, {}, clear=True):
|
||||||
|
assert _env_int("X_MS", fallback) == fallback
|
||||||
|
for bad in ("", " ", "abc", "0", "-5"):
|
||||||
|
with patch.dict(os.environ, {"X_MS": bad}):
|
||||||
|
assert _env_int("X_MS", fallback) == fallback
|
||||||
|
|||||||
@@ -197,7 +197,9 @@ async def test_grok_spawn_wires_gateway_and_image_last() -> None:
|
|||||||
# Gateway + operational env the grok image entrypoint consumes.
|
# Gateway + operational env the grok image entrypoint consumes.
|
||||||
assert "ROBOCO_MCP_CONFIG=/app/mcp-config.json" in cmd
|
assert "ROBOCO_MCP_CONFIG=/app/mcp-config.json" in cmd
|
||||||
assert "ROBOCO_SYSTEM_PROMPT=/app/system-prompt.md" in cmd
|
assert "ROBOCO_SYSTEM_PROMPT=/app/system-prompt.md" in cmd
|
||||||
assert "ROBOCO_AGENT_TOOLS=Read,Write,Edit,Bash,Grep,Glob,TodoWrite" in cmd
|
# Tool restriction lives in the rendered opencode.json (opencode `tools`),
|
||||||
|
# not a spawn env var — no ROBOCO_AGENT_TOOLS is injected.
|
||||||
|
assert not any(c.startswith("ROBOCO_AGENT_TOOLS=") for c in cmd)
|
||||||
# Identity wiring from the shared host helpers is present.
|
# Identity wiring from the shared host helpers is present.
|
||||||
assert "ROBOCO_AGENT_TOKEN=hmac-be-dev-1" in cmd
|
assert "ROBOCO_AGENT_TOKEN=hmac-be-dev-1" in cmd
|
||||||
# The image is the final docker-run argument.
|
# The image is the final docker-run argument.
|
||||||
|
|||||||
Reference in New Issue
Block a user