diff --git a/roboco/llm/providers/grok.py b/roboco/llm/providers/grok.py index fc913b23..f5a24d0b 100644 --- a/roboco/llm/providers/grok.py +++ b/roboco/llm/providers/grok.py @@ -44,6 +44,7 @@ from typing import TYPE_CHECKING, Protocol from roboco.agents_config import get_agent_role from roboco.llm.providers._docker import container_running, stop_container from roboco.llm.providers.base import AgentProvider, ProviderError, SpawnResult +from roboco.services.gateway.role_config import get_role_config if TYPE_CHECKING: from pathlib import Path @@ -90,6 +91,42 @@ _MINIMAL_REASONING_ROLES = frozenset( ) _FULL_REASONING_OVERRIDES = frozenset({"default", "full", "none", ""}) +# Per-role opencode permission policy (Claude-parity with +# orchestrator._get_role_permissions). Claude denies Write/Edit for the read-only +# roles and Bash(git commit/push) for PMs; opencode's permission is coarser +# (allow/deny per tool class), so: +# * edit — allow only roles that write code (role_config.allows_write: +# developer / documenter). Everyone else edit=deny. +# * bash — allow only roles that legitimately run a shell; the read-only +# reviewers (qa / pr_reviewer / auditor) and the board never do. secret-scrub +# still guards bash (git-mutate / cred files) for the roles that keep it. +# * external_directory — only the pr_reviewer reads scratch outside its cwd +# (a diff it writes to /tmp). Delivery roles work inside their workspace, so +# external_directory=deny (the headless-ask auto-deny that blocked the +# pr-reviewer is moot once that one role is explicitly allowed). +_BASH_ROLES = frozenset({"developer", "documenter", "cell_pm", "main_pm"}) +_EXTERNAL_DIR_ROLES = frozenset({"pr_reviewer"}) + + +def _edit_permission_for(agent_id: str) -> str: + """opencode ``edit`` permission for an agent's role (allow iff it writes code).""" + role = get_agent_role(agent_id) or "" + try: + return "allow" if get_role_config(role).allows_write else "deny" + except KeyError: + return "deny" # unknown role → safest + + +def _bash_permission_for(agent_id: str) -> str: + """opencode ``bash`` permission for an agent's role.""" + return "allow" if (get_agent_role(agent_id) or "") in _BASH_ROLES else "deny" + + +def _external_dir_permission_for(agent_id: str) -> str: + """opencode ``external_directory`` permission for an agent's role.""" + role = get_agent_role(agent_id) or "" + return "allow" if role in _EXTERNAL_DIR_ROLES else "deny" + def _reasoning_effort_for(agent_id: str) -> str | None: """Resolve the opencode --variant reasoning effort for an agent. @@ -243,6 +280,20 @@ class GrokProvider(AgentProvider): # Reused as the generic agent session id so the transcript stays # locatable at finalize, exactly as on the Claude Code path. cmd.extend(["-e", f"ROBOCO_AGENT_SESSION_ID={config.claude_session_id}"]) + # Per-role opencode permissions (Claude-parity): read-only roles get + # edit=deny, only delivery roles get bash, only the pr-reviewer gets + # external-directory reads. opencode_config.main() reads these. + cmd.extend( + [ + "-e", + f"ROBOCO_GROK_EDIT_PERMISSION={_edit_permission_for(config.agent_id)}", + "-e", + f"ROBOCO_GROK_BASH_PERMISSION={_bash_permission_for(config.agent_id)}", + "-e", + "ROBOCO_GROK_EXTERNAL_DIR_PERMISSION=" + f"{_external_dir_permission_for(config.agent_id)}", + ] + ) # Reasoning effort (opencode --variant) by role; omitted = full reasoning. variant = _reasoning_effort_for(config.agent_id) if variant: diff --git a/roboco/runtime/orchestrator.py b/roboco/runtime/orchestrator.py index 381e4071..14f37878 100644 --- a/roboco/runtime/orchestrator.py +++ b/roboco/runtime/orchestrator.py @@ -3415,6 +3415,23 @@ class AgentOrchestrator: "ROBOCO_SYSTEM_PROMPT=/app/system-prompt.md", ] ) + # Both interactive roles are read-only conversational agents: no code + # edits, no shell (Claude-parity — the SDK gates deny everything but + # Read/Grep/Glob + their tools). Intake reads sibling product repos + # that sit OUTSIDE its cwd, so it keeps external-directory reads; the + # Secretary only reads /app + the API, so it doesn't. + is_intake = isinstance(spec, _IntakeRunSpec) + cmd.extend( + [ + "-e", + "ROBOCO_GROK_EDIT_PERMISSION=deny", + "-e", + "ROBOCO_GROK_BASH_PERMISSION=deny", + "-e", + "ROBOCO_GROK_EXTERNAL_DIR_PERMISSION=" + f"{'allow' if is_intake else 'deny'}", + ] + ) # 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: diff --git a/tests/unit/llm/test_providers.py b/tests/unit/llm/test_providers.py index c72d424f..beb1b6d0 100644 --- a/tests/unit/llm/test_providers.py +++ b/tests/unit/llm/test_providers.py @@ -23,7 +23,12 @@ from roboco.llm.providers import ( ProviderRegistry, SpawnResult, ) -from roboco.llm.providers.grok import _reasoning_effort_for +from roboco.llm.providers.grok import ( + _bash_permission_for, + _edit_permission_for, + _external_dir_permission_for, + _reasoning_effort_for, +) from roboco.models.base import ModelProvider from roboco.models.runtime import OrchestratorAgentConfig @@ -279,6 +284,59 @@ def test_reasoning_effort_override(monkeypatch: pytest.MonkeyPatch) -> None: assert _reasoning_effort_for("be-pm") is None # "default" => full reasoning +# --------------------------------------------------------------------------- +# Per-role opencode permissions (Claude-parity) +# --------------------------------------------------------------------------- + + +def test_edit_permission_allows_only_writer_roles() -> None: + assert _edit_permission_for("be-dev-1") == "allow" + assert _edit_permission_for("be-doc") == "allow" + for slug in ("be-qa", "pr-reviewer-1", "be-pm", "main-pm", "auditor"): + assert _edit_permission_for(slug) == "deny", slug + + +def test_bash_permission_allows_only_shell_roles() -> None: + for slug in ("be-dev-1", "be-doc", "be-pm", "main-pm"): + assert _bash_permission_for(slug) == "allow", slug + for slug in ("be-qa", "pr-reviewer-1", "auditor", "product-owner"): + assert _bash_permission_for(slug) == "deny", slug + + +def test_external_dir_permission_only_pr_reviewer() -> None: + assert _external_dir_permission_for("pr-reviewer-1") == "allow" + for slug in ("be-dev-1", "be-qa", "be-pm", "auditor"): + assert _external_dir_permission_for(slug) == "deny", slug + + +async def test_grok_spawn_sets_readonly_permissions_for_reviewer() -> None: + # A read-only reviewer (qa) gets edit=deny + bash=deny + external_dir=deny. + host = _FakeHost() + provider = GrokProvider(host) + with patch( + "asyncio.create_subprocess_exec", AsyncMock(return_value=_proc()) + ) as exec_mock: + await provider.spawn(_config(agent_id="be-qa")) + cmd = list(exec_mock.call_args.args) + assert "ROBOCO_GROK_EDIT_PERMISSION=deny" in cmd + assert "ROBOCO_GROK_BASH_PERMISSION=deny" in cmd + assert "ROBOCO_GROK_EXTERNAL_DIR_PERMISSION=deny" in cmd + + +async def test_grok_spawn_pr_reviewer_is_read_only_but_reads_scratch() -> None: + # The pr-reviewer never writes code (edit=deny) but reads its /tmp diff + # (external_directory=allow) — the one role that needs it. + host = _FakeHost() + provider = GrokProvider(host) + with patch( + "asyncio.create_subprocess_exec", AsyncMock(return_value=_proc()) + ) as exec_mock: + await provider.spawn(_config(agent_id="pr-reviewer-1")) + cmd = list(exec_mock.call_args.args) + assert "ROBOCO_GROK_EDIT_PERMISSION=deny" in cmd + assert "ROBOCO_GROK_EXTERNAL_DIR_PERMISSION=allow" in cmd + + async def test_grok_spawn_sets_variant_for_minimal_role() -> None: host = _FakeHost() provider = GrokProvider(host) diff --git a/tests/unit/runtime/test_interactive_grok_spawn.py b/tests/unit/runtime/test_interactive_grok_spawn.py index 406ab7d9..40c015bb 100644 --- a/tests/unit/runtime/test_interactive_grok_spawn.py +++ b/tests/unit/runtime/test_interactive_grok_spawn.py @@ -67,6 +67,21 @@ def test_intake_grok_uses_openai_env_and_opencode_mount() -> None: assert cmd[-1] == GROK_PROMPTER_IMAGE # The xAI endpoint is never mislabelled as Anthropic. assert not any(c.startswith("ANTHROPIC_") for c in cmd) + # Intake is read-only (no code edits, no shell) but reads sibling product + # repos OUTSIDE its cwd, so it keeps external-directory reads. + assert "ROBOCO_GROK_EDIT_PERMISSION=deny" in cmd + assert "ROBOCO_GROK_BASH_PERMISSION=deny" in cmd + assert "ROBOCO_GROK_EXTERNAL_DIR_PERMISSION=allow" in cmd + + +def test_intake_anthropic_omits_grok_permission_env() -> None: + # The opencode permission env is a GROK-only contract; the Claude path never + # sets it (it gates tools via the SDK can_use_tool allowlist instead). + cmd = AgentOrchestrator._build_intake_run_cmd( + _intake_spec("anthropic", base_url="https://api.anthropic.com", token="sk-ant") + ) + assert not any(c.startswith("ROBOCO_GROK_EDIT_PERMISSION=") for c in cmd) + assert not any(c.startswith("ROBOCO_GROK_BASH_PERMISSION=") for c in cmd) def test_intake_grok_omits_variant_when_unset() -> None: @@ -109,3 +124,8 @@ def test_secretary_grok_uses_openai_env_and_grok_image() -> None: assert "ROBOCO_AGENT_TOKEN=hmac-secretary" in cmd assert cmd[-1] == GROK_SECRETARY_IMAGE assert not any(c.startswith("ANTHROPIC_") for c in cmd) + # The Secretary is read-only and reads only /app + the API, so edit/bash + # are denied and it gets NO external-directory reads (unlike intake). + assert "ROBOCO_GROK_EDIT_PERMISSION=deny" in cmd + assert "ROBOCO_GROK_BASH_PERMISSION=deny" in cmd + assert "ROBOCO_GROK_EXTERNAL_DIR_PERMISSION=deny" in cmd