diff --git a/roboco/agent_sdk/opencode_session.py b/roboco/agent_sdk/opencode_session.py index 6597cecd..b5e59615 100644 --- a/roboco/agent_sdk/opencode_session.py +++ b/roboco/agent_sdk/opencode_session.py @@ -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 diff --git a/roboco/llm/providers/grok.py b/roboco/llm/providers/grok.py index fd0725a4..fc913b23 100644 --- a/roboco/llm/providers/grok.py +++ b/roboco/llm/providers/grok.py @@ -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 diff --git a/roboco/runtime/orchestrator.py b/roboco/runtime/orchestrator.py index dcfd359d..d84907fe 100644 --- a/roboco/runtime/orchestrator.py +++ b/roboco/runtime/orchestrator.py @@ -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}"]) diff --git a/tests/unit/agent_sdk/test_opencode_session.py b/tests/unit/agent_sdk/test_opencode_session.py index a8012de1..4cb46ad4 100644 --- a/tests/unit/agent_sdk/test_opencode_session.py +++ b/tests/unit/agent_sdk/test_opencode_session.py @@ -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: diff --git a/tests/unit/llm/test_providers.py b/tests/unit/llm/test_providers.py index a9e34595..c72d424f 100644 --- a/tests/unit/llm/test_providers.py +++ b/tests/unit/llm/test_providers.py @@ -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]: diff --git a/tests/unit/runtime/test_interactive_grok_spawn.py b/tests/unit/runtime/test_interactive_grok_spawn.py index f9b78a1f..406ab7d9 100644 --- a/tests/unit/runtime/test_interactive_grok_spawn.py +++ b/tests/unit/runtime/test_interactive_grok_spawn.py @@ -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")