mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(grok): make the live interactive path work — store perms, error surfacing, variant
Found by actually running opencode serve locally (the path was doc-verified but never executed). Three fixes: 1. EACCES on the opencode store mount (the live intake crash): on Linux docker auto-creates a missing bind source as root:root, so the non-root agent user could not mkdir/write in /home/agent/.local/share/opencode and opencode died at boot. _ensure_opencode_data_dir pre-creates the per-agent dir 0777 before the mount (one-shot via the _GrokHost seam, interactive in both spawns). 2. Silent blank reply on a model error: opencode reports a turn failure in info.error with parts=[], NOT as a part — verified live (a bad xAI key returns info.error APIError). send() / normalize_opencode_message now surface it as an "error" StreamChunk so a failed turn is never blank (the original intake bug class). Confirmed live: the error now renders. 3. Reasoning variant on the serve path: the live OpenAPI shows the message body accepts a "variant" field (it is NOT CLI-only, as the docs implied), so the pin is unblocked. send() passes ROBOCO_GROK_VARIANT as the per-turn variant; the orchestrator sets it per-role (_reasoning_effort_for) for interactive Grok, the same lever as the one-shot --variant. opencode serve startup, POST /session, session-id extraction, the part-type mapping (text/reasoning/tool), and the error path are all validated against a live opencode 1.17.8. A real successful grok reply still needs a funded key.
This commit is contained in:
@@ -76,19 +76,46 @@ def _part_to_chunk(
|
||||
return None, None, None
|
||||
|
||||
|
||||
def normalize_opencode_message(parts: list[dict[str, Any]]) -> list[StreamChunk]:
|
||||
"""Map an opencode message's parts to panel chunks (+ draft + turn_end).
|
||||
def _message_error(message: dict[str, Any]) -> str | None:
|
||||
"""Human-readable text of a turn-level error (``info.error``), else ``None``.
|
||||
|
||||
A model/turn failure (bad key, rate limit, model error) is reported by
|
||||
opencode in ``info.error`` with an EMPTY ``parts`` list — not as a part — so
|
||||
it must be surfaced explicitly or the turn renders blank (the original Claude
|
||||
intake bug). Confirmed live: a bad xAI key returns
|
||||
``info.error={"name":"APIError","data":{"message":"Incorrect API key ..."}}``.
|
||||
"""
|
||||
info = message.get("info")
|
||||
if not isinstance(info, dict):
|
||||
return None
|
||||
err = info.get("error")
|
||||
if not err:
|
||||
return None
|
||||
if isinstance(err, dict):
|
||||
data = err.get("data")
|
||||
if isinstance(data, dict) and data.get("message"):
|
||||
return str(data["message"])
|
||||
if err.get("name"):
|
||||
return str(err["name"])
|
||||
return str(err)
|
||||
|
||||
|
||||
def normalize_opencode_message(message: dict[str, Any]) -> list[StreamChunk]:
|
||||
"""Map an opencode message reply (``{info, parts}``) to panel chunks.
|
||||
|
||||
Unlike the Claude path (which streams text deltas live and so drops the final
|
||||
TextBlock to avoid double-render), the synchronous opencode reply carries the
|
||||
text only here, so text parts ARE emitted. A ``propose_draft`` tool part — or
|
||||
a fenced ```roboco-draft``` block in the assembled text — becomes a ``draft``
|
||||
chunk, matching the Claude intake's two draft paths.
|
||||
chunk, matching the Claude intake's two draft paths. A turn-level
|
||||
``info.error`` is surfaced as an ``error`` chunk so a failed turn is never
|
||||
silently blank.
|
||||
"""
|
||||
parts = message.get("parts") or []
|
||||
chunks: list[StreamChunk] = []
|
||||
text_parts: list[str] = []
|
||||
draft: dict[str, Any] | None = None
|
||||
for part in parts or []:
|
||||
for part in parts:
|
||||
chunk, text_part, block_draft = _part_to_chunk(part)
|
||||
if chunk is not None:
|
||||
chunks.append(chunk)
|
||||
@@ -99,6 +126,9 @@ def normalize_opencode_message(parts: list[dict[str, Any]]) -> list[StreamChunk]
|
||||
draft = draft or _extract_draft("".join(text_parts))
|
||||
if draft is not None:
|
||||
chunks.append(StreamChunk(kind="draft", data=draft))
|
||||
error = _message_error(message)
|
||||
if error:
|
||||
chunks.append(StreamChunk(kind="error", text=error))
|
||||
chunks.append(StreamChunk(kind="turn_end", data={}))
|
||||
return chunks
|
||||
|
||||
@@ -176,18 +206,26 @@ class OpencodeServeSession:
|
||||
"""Run one turn (synchronous message) and yield its normalized chunks."""
|
||||
if self._client is None or self._session_id is None:
|
||||
raise RuntimeError("OpencodeServeSession used outside its context")
|
||||
body: dict[str, Any] = {"parts": [{"type": "text", "text": text}]}
|
||||
# Per-role reasoning effort: the orchestrator sets ROBOCO_GROK_VARIANT on
|
||||
# the container; the serve message endpoint accepts a `variant` field
|
||||
# (confirmed against the live opencode OpenAPI), the same lever the
|
||||
# one-shot path drives via `opencode run --variant`.
|
||||
variant = _variant()
|
||||
if variant:
|
||||
body["variant"] = variant
|
||||
try:
|
||||
resp = await self._client.post(
|
||||
f"{self._base}/session/{self._session_id}/message",
|
||||
json={"parts": [{"type": "text", "text": text}]},
|
||||
json=body,
|
||||
)
|
||||
resp.raise_for_status()
|
||||
parts = resp.json().get("parts", [])
|
||||
message = resp.json()
|
||||
except Exception as exc:
|
||||
logger.error("opencode message turn failed", error=str(exc))
|
||||
yield StreamChunk(kind="error", text=str(exc))
|
||||
return
|
||||
for chunk in normalize_opencode_message(parts):
|
||||
for chunk in normalize_opencode_message(message):
|
||||
yield chunk
|
||||
|
||||
|
||||
@@ -213,3 +251,14 @@ def serve_port() -> int:
|
||||
if raw.isdigit() and int(raw) > 0:
|
||||
return int(raw)
|
||||
return _DEFAULT_PORT
|
||||
|
||||
|
||||
def _variant() -> str | None:
|
||||
"""The opencode reasoning variant to apply per turn (ROBOCO_GROK_VARIANT).
|
||||
|
||||
Set by the orchestrator from the per-role reasoning-effort policy (the same
|
||||
value the one-shot path passes to ``opencode run --variant``); unset = the
|
||||
model's default (full) reasoning.
|
||||
"""
|
||||
raw = os.environ.get("ROBOCO_GROK_VARIANT", "").strip()
|
||||
return raw or None
|
||||
|
||||
@@ -117,6 +117,8 @@ class _GrokHost(Protocol):
|
||||
|
||||
async def _remove_container(self, container_name: str) -> None: ...
|
||||
|
||||
def _ensure_opencode_data_dir(self, agent_id: str) -> None: ...
|
||||
|
||||
def _resolve_host_paths(
|
||||
self, config: AgentConfig, agent_settings_path: Path | None
|
||||
) -> dict[str, str | None]: ...
|
||||
@@ -160,6 +162,9 @@ class GrokProvider(AgentProvider):
|
||||
|
||||
container_name = _container_name(config.agent_id)
|
||||
await self._host._remove_container(container_name)
|
||||
# Pre-create the opencode store dir (world-writable) before the bind mount
|
||||
# so the non-root agent user can write opencode.db / repos (else EACCES).
|
||||
self._host._ensure_opencode_data_dir(config.agent_id)
|
||||
|
||||
# Reuse the orchestrator's mount/auth/git assembly so the agent gets the
|
||||
# full MCP gateway + identity wiring. Blank the provider routing fields
|
||||
|
||||
@@ -211,6 +211,7 @@ class _IntakeRunSpec:
|
||||
provider_auth_token: str | None
|
||||
provider_type: str = "anthropic"
|
||||
model: str = ""
|
||||
grok_variant: str | None = None
|
||||
|
||||
|
||||
@dataclass
|
||||
@@ -234,6 +235,7 @@ class _SecretaryRunSpec:
|
||||
provider_auth_token: str | None
|
||||
provider_type: str = "anthropic"
|
||||
model: str = ""
|
||||
grok_variant: str | None = None
|
||||
|
||||
|
||||
def _read_project_slug(task: dict[str, Any]) -> str | None:
|
||||
@@ -868,6 +870,32 @@ class AgentOrchestrator:
|
||||
img, f"{docker_dir}/{dockerfile}", build_context
|
||||
)
|
||||
|
||||
def _ensure_opencode_data_dir(self, agent_id: str) -> None:
|
||||
"""Pre-create the agent's opencode store dir (world-writable) before the mount.
|
||||
|
||||
On Linux, ``docker run -v`` auto-creates a MISSING bind source as
|
||||
``root:root``, so the non-root ``agent`` user EACCESes on opencode's
|
||||
first write (``repos/``, ``opencode.db``) and ``opencode serve``/``run``
|
||||
dies at boot — the live intake crash. Creating the dir ``0777`` first
|
||||
makes the mounted store writable regardless of the agent uid; the
|
||||
orchestrator (root) can still read it back at finalize. Mirrors the
|
||||
container-vs-local split in ``_resolve_host_paths``.
|
||||
"""
|
||||
if PROJECT_HOST_PATH:
|
||||
target = Path(OPENCODE_DATA_DIR) / agent_id
|
||||
else:
|
||||
target = Path(tempfile.gettempdir()) / "roboco-opencode" / agent_id
|
||||
try:
|
||||
target.mkdir(parents=True, exist_ok=True)
|
||||
target.chmod(0o777)
|
||||
except OSError as exc:
|
||||
logger.warning(
|
||||
"could not pre-create opencode data dir; grok agent may EACCES",
|
||||
agent_id=agent_id,
|
||||
path=str(target),
|
||||
error=str(exc),
|
||||
)
|
||||
|
||||
async def _ensure_image_present(
|
||||
self, bare_image: str, dockerfile_path: str, build_context: str
|
||||
) -> None:
|
||||
@@ -2975,8 +3003,13 @@ class AgentOrchestrator:
|
||||
# other provider uses the Claude SDK-driver prompter image.
|
||||
is_grok = route.provider_type == ModelProvider.GROK
|
||||
image = GROK_PROMPTER_IMAGE if is_grok else get_agent_image(INTAKE_AGENT_ID)
|
||||
grok_variant: str | None = None
|
||||
if is_grok:
|
||||
from roboco.llm.providers.grok import _reasoning_effort_for
|
||||
|
||||
grok_variant = _reasoning_effort_for(INTAKE_AGENT_ID)
|
||||
await self._ensure_grok_interactive_image(image)
|
||||
self._ensure_opencode_data_dir(INTAKE_AGENT_ID)
|
||||
else:
|
||||
await self._ensure_agent_image(INTAKE_AGENT_ID)
|
||||
container_name = f"roboco-agent-{INTAKE_AGENT_ID}"
|
||||
@@ -2995,6 +3028,7 @@ class AgentOrchestrator:
|
||||
provider_auth_token=route.auth_token,
|
||||
provider_type=route.provider_type.value,
|
||||
model=route.model_name,
|
||||
grok_variant=grok_variant,
|
||||
)
|
||||
)
|
||||
container_id = await self._run_container_cmd(cmd)
|
||||
@@ -3129,8 +3163,13 @@ class AgentOrchestrator:
|
||||
|
||||
is_grok = route.provider_type == ModelProvider.GROK
|
||||
image = GROK_SECRETARY_IMAGE if is_grok else get_agent_image(SECRETARY_AGENT_ID)
|
||||
grok_variant: str | None = None
|
||||
if is_grok:
|
||||
from roboco.llm.providers.grok import _reasoning_effort_for
|
||||
|
||||
grok_variant = _reasoning_effort_for(SECRETARY_AGENT_ID)
|
||||
await self._ensure_grok_interactive_image(image)
|
||||
self._ensure_opencode_data_dir(SECRETARY_AGENT_ID)
|
||||
else:
|
||||
await self._ensure_agent_image(SECRETARY_AGENT_ID)
|
||||
container_name = f"roboco-agent-{SECRETARY_AGENT_ID}"
|
||||
@@ -3152,6 +3191,7 @@ class AgentOrchestrator:
|
||||
provider_auth_token=route.auth_token,
|
||||
provider_type=route.provider_type.value,
|
||||
model=route.model_name,
|
||||
grok_variant=grok_variant,
|
||||
)
|
||||
)
|
||||
container_id = await self._run_container_cmd(cmd)
|
||||
@@ -3375,6 +3415,10 @@ class AgentOrchestrator:
|
||||
"ROBOCO_SYSTEM_PROMPT=/app/system-prompt.md",
|
||||
]
|
||||
)
|
||||
# Per-role reasoning effort: the opencode-serve driver passes this as
|
||||
# the message `variant` (same lever as the one-shot --variant).
|
||||
if spec.grok_variant:
|
||||
cmd.extend(["-e", f"ROBOCO_GROK_VARIANT={spec.grok_variant}"])
|
||||
return
|
||||
if base_url:
|
||||
cmd.extend(["-e", f"ANTHROPIC_BASE_URL={base_url}"])
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
"""normalize_opencode_message maps opencode message parts to panel chunks.
|
||||
"""normalize_opencode_message maps an opencode message reply to panel chunks.
|
||||
|
||||
The OpencodeServeSession transport (subprocess + HTTP) needs a live opencode,
|
||||
like the Claude SdkIntakeSession; the deterministic part→chunk mapping and the
|
||||
session-id extraction are covered here.
|
||||
The OpencodeServeSession transport (subprocess + HTTP) is exercised live against
|
||||
a real `opencode serve`; the deterministic message→chunk mapping, the
|
||||
turn-level error surfacing, and session-id extraction are covered here.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -11,6 +11,7 @@ from typing import TYPE_CHECKING
|
||||
|
||||
from roboco.agent_sdk.opencode_session import (
|
||||
_extract_session_id,
|
||||
_message_error,
|
||||
normalize_opencode_message,
|
||||
)
|
||||
|
||||
@@ -23,20 +24,20 @@ def _kinds(chunks: list[StreamChunk]) -> list[str]:
|
||||
|
||||
|
||||
def test_text_part_emits_text_then_turn_end() -> None:
|
||||
chunks = normalize_opencode_message([{"type": "text", "text": "Hi there"}])
|
||||
chunks = normalize_opencode_message({"parts": [{"type": "text", "text": "Hi"}]})
|
||||
assert _kinds(chunks) == ["text", "turn_end"]
|
||||
assert chunks[0].text == "Hi there"
|
||||
assert chunks[0].text == "Hi"
|
||||
|
||||
|
||||
def test_reasoning_part_maps_to_thinking() -> None:
|
||||
chunks = normalize_opencode_message([{"type": "reasoning", "text": "hmm"}])
|
||||
chunks = normalize_opencode_message({"parts": [{"type": "reasoning", "text": "x"}]})
|
||||
assert chunks[0].kind == "thinking"
|
||||
assert chunks[0].text == "hmm"
|
||||
assert chunks[0].text == "x"
|
||||
|
||||
|
||||
def test_tool_part_maps_to_tool_use() -> None:
|
||||
chunks = normalize_opencode_message(
|
||||
[{"type": "tool", "tool": "read", "input": {"path": "x"}}]
|
||||
{"parts": [{"type": "tool", "tool": "read", "input": {"path": "x"}}]}
|
||||
)
|
||||
tool = next(c for c in chunks if c.kind == "tool_use")
|
||||
assert tool.tool == "read"
|
||||
@@ -45,18 +46,42 @@ def test_tool_part_maps_to_tool_use() -> None:
|
||||
|
||||
def test_fenced_draft_in_text_becomes_draft_chunk() -> None:
|
||||
fenced = '```roboco-draft\n{"title": "Add login"}\n```'
|
||||
chunks = normalize_opencode_message([{"type": "text", "text": fenced}])
|
||||
chunks = normalize_opencode_message({"parts": [{"type": "text", "text": fenced}]})
|
||||
draft = next(c for c in chunks if c.kind == "draft")
|
||||
assert draft.data["title"] == "Add login"
|
||||
|
||||
|
||||
def test_unknown_part_skipped_but_turn_still_ends() -> None:
|
||||
chunks = normalize_opencode_message([{"type": "mystery", "x": 1}])
|
||||
chunks = normalize_opencode_message({"parts": [{"type": "mystery", "x": 1}]})
|
||||
assert _kinds(chunks) == ["turn_end"]
|
||||
|
||||
|
||||
def test_empty_message_yields_only_turn_end() -> None:
|
||||
assert _kinds(normalize_opencode_message([])) == ["turn_end"]
|
||||
assert _kinds(normalize_opencode_message({"parts": []})) == ["turn_end"]
|
||||
|
||||
|
||||
def test_turn_level_error_is_surfaced_not_blank() -> None:
|
||||
# A model failure lands in info.error with parts=[]; it must NOT render blank.
|
||||
msg = {
|
||||
"info": {
|
||||
"role": "assistant",
|
||||
"error": {
|
||||
"name": "APIError",
|
||||
"data": {"message": "Incorrect API key provided"},
|
||||
},
|
||||
},
|
||||
"parts": [],
|
||||
}
|
||||
chunks = normalize_opencode_message(msg)
|
||||
assert _kinds(chunks) == ["error", "turn_end"]
|
||||
assert "Incorrect API key" in chunks[0].text
|
||||
|
||||
|
||||
def test_message_error_extraction() -> None:
|
||||
assert _message_error({"info": {"error": {"data": {"message": "boom"}}}}) == "boom"
|
||||
assert _message_error({"info": {"error": {"name": "APIError"}}}) == "APIError"
|
||||
assert _message_error({"info": {}}) is None
|
||||
assert _message_error({"parts": []}) is None
|
||||
|
||||
|
||||
def test_extract_session_id_is_tolerant() -> None:
|
||||
|
||||
@@ -55,6 +55,7 @@ class _FakeHost:
|
||||
self.removed: list[str] = []
|
||||
self.spawn_args: tuple[object, ...] | None = None
|
||||
self.mount_config: OrchestratorAgentConfig | None = None
|
||||
self.opencode_dirs_ensured: list[str] = []
|
||||
|
||||
async def _spawn_container(
|
||||
self,
|
||||
@@ -68,6 +69,9 @@ class _FakeHost:
|
||||
async def _remove_container(self, container_name: str) -> None:
|
||||
self.removed.append(container_name)
|
||||
|
||||
def _ensure_opencode_data_dir(self, agent_id: str) -> None:
|
||||
self.opencode_dirs_ensured.append(agent_id)
|
||||
|
||||
def _resolve_host_paths(
|
||||
self, config: OrchestratorAgentConfig, agent_settings_path: Path | None
|
||||
) -> dict[str, str | None]:
|
||||
|
||||
@@ -24,7 +24,11 @@ _HOSTS: dict[str, str | None] = {
|
||||
|
||||
|
||||
def _intake_spec(
|
||||
provider_type: str, *, base_url: str | None, token: str | None
|
||||
provider_type: str,
|
||||
*,
|
||||
base_url: str | None,
|
||||
token: str | None,
|
||||
grok_variant: str | None = None,
|
||||
) -> _IntakeRunSpec:
|
||||
return _IntakeRunSpec(
|
||||
container_name="roboco-agent-intake-1",
|
||||
@@ -40,23 +44,38 @@ def _intake_spec(
|
||||
provider_auth_token=token,
|
||||
provider_type=provider_type,
|
||||
model="grok-build-0.1",
|
||||
grok_variant=grok_variant,
|
||||
)
|
||||
|
||||
|
||||
def test_intake_grok_uses_openai_env_and_opencode_mount() -> None:
|
||||
cmd = AgentOrchestrator._build_intake_run_cmd(
|
||||
_intake_spec("grok", base_url="https://api.x.ai/v1", token="xai-key")
|
||||
_intake_spec(
|
||||
"grok",
|
||||
base_url="https://api.x.ai/v1",
|
||||
token="xai-key",
|
||||
grok_variant="minimal",
|
||||
)
|
||||
)
|
||||
assert "OPENAI_BASE_URL=https://api.x.ai/v1" in cmd
|
||||
assert "OPENAI_API_KEY=xai-key" in cmd
|
||||
assert "ROBOCO_AGENT_MODEL=grok-build-0.1" in cmd
|
||||
assert "ROBOCO_SYSTEM_PROMPT=/app/system-prompt.md" in cmd
|
||||
assert "/h/oc/intake-1:/home/agent/.local/share/opencode" in cmd
|
||||
# Per-role reasoning effort reaches the container for the serve driver.
|
||||
assert "ROBOCO_GROK_VARIANT=minimal" in cmd
|
||||
assert cmd[-1] == GROK_PROMPTER_IMAGE
|
||||
# The xAI endpoint is never mislabelled as Anthropic.
|
||||
assert not any(c.startswith("ANTHROPIC_") for c in cmd)
|
||||
|
||||
|
||||
def test_intake_grok_omits_variant_when_unset() -> None:
|
||||
cmd = AgentOrchestrator._build_intake_run_cmd(
|
||||
_intake_spec("grok", base_url="https://api.x.ai/v1", token="xai-key")
|
||||
)
|
||||
assert not any(c.startswith("ROBOCO_GROK_VARIANT=") for c in cmd)
|
||||
|
||||
|
||||
def test_intake_anthropic_keeps_anthropic_env() -> None:
|
||||
cmd = AgentOrchestrator._build_intake_run_cmd(
|
||||
_intake_spec("anthropic", base_url="https://api.anthropic.com", token="sk-ant")
|
||||
|
||||
Reference in New Issue
Block a user