From d1d1b638ae0a95ca9618af5eb66d3cd836b40517 Mon Sep 17 00:00:00 2001 From: Renn F Date: Fri, 19 Jun 2026 00:44:42 +0200 Subject: [PATCH] fix(grok): unbreak workspace-cwd agents, free trapped agents, stop self-PR review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bugs surfaced by the first live Grok lifecycle run: - Dev/QA/doc agents crash-looped at startup with ModuleNotFoundError on roboco.llm.providers. The entrypoint ran the opencode-config render from the agent's workspace-clone cwd, whose own roboco/ dir shadows /app on the sys.path front; a branch without the grok code lacks the providers package. Render from /app so the installed package always resolves (the render has no cwd dependency — writes global, reads ROBOCO_MCP_CONFIG). - A budget/loop halt blocked EVERY tool, including i_am_idle, unclaim, and i_am_blocked, so a halted agent could neither continue nor stop and flailed — one billed model turn per blocked retry. The before-gate now always lets the release verbs through so a halted agent can exit cleanly. - The inbound reviewer ingested the org's OWN PRs (authored by the repo-owner account), which can't take a REQUEST_CHANGES review (GitHub 422) and get re-reviewed every poll. The normalizer flags author_is_owner and ingestion skips them — the reviewer reviews only PRs the org did not author. External/contributor PRs are unaffected. Tests: owner-authored PR flagged + skipped; normalize shape covers the new field. Gate green on the changed modules (ruff/mypy/xenon + 48 tests). --- docker/grok/budget-feed.js | 9 ++++++++ docker/scripts/grok-agent-entrypoint.sh | 7 ++++++- roboco/runtime/orchestrator.py | 5 +++++ roboco/services/git.py | 11 +++++++++- .../unit/runtime/test_external_pr_classify.py | 21 +++++++++++++++++++ tests/unit/services/test_git_inbound_pr.py | 20 ++++++++++++++++++ 6 files changed, 71 insertions(+), 2 deletions(-) diff --git a/docker/grok/budget-feed.js b/docker/grok/budget-feed.js index 257346f8..b772a9c7 100644 --- a/docker/grok/budget-feed.js +++ b/docker/grok/budget-feed.js @@ -81,12 +81,21 @@ function bareVerb(tool) { return tool; } +// Release/escape verbs must ALWAYS be allowed through the before-gate. A halt +// (budget, loop, or fail-closed) that also blocks these traps the agent: it can +// neither continue nor stop, so it flails — and every blocked retry is another +// billed model turn. Letting i_am_idle / unclaim / i_am_blocked through is the +// only way a halted agent can exit cleanly. +const RELEASE_VERBS = new Set(["i_am_idle", "unclaim", "i_am_blocked"]); + // Named export (opencode's plugin convention) + baked into the plugin // auto-discovery dir (~/.config/opencode/plugin/) at image build — the simplest // registration route (no config `plugin:` path needed). export const RobocoBudgetFeed = async () => { return { "tool.execute.before": async (input) => { + // Escape hatches always pass — a halted agent must be able to stop. + if (RELEASE_VERBS.has(bareVerb(String(input?.tool || "")))) return; const status = await sdk("GET", "/budget/status", null); if (!status) { // One-shot delivery agents MUST have the in-container SDK budget server diff --git a/docker/scripts/grok-agent-entrypoint.sh b/docker/scripts/grok-agent-entrypoint.sh index a0d67a78..7a3a19e6 100755 --- a/docker/scripts/grok-agent-entrypoint.sh +++ b/docker/scripts/grok-agent-entrypoint.sh @@ -15,7 +15,12 @@ SDK_URL="http://localhost:${SDK_PORT}" # Generate opencode.json (provider + model + MCP gateway + permissions + # instructions). Writes to opencode's global config dir by default. -python -m roboco.llm.providers.opencode_config +# Run from /app so `python -m` resolves the INSTALLED roboco package. Dev/doc/qa +# agents run at their workspace-clone cwd, which has its own `roboco/` dir on the +# sys.path front (python -m prepends cwd); on a branch without the grok code that +# clone lacks roboco.llm.providers and shadows /app → ModuleNotFoundError. The +# config render has no cwd dependency (writes global, reads ROBOCO_MCP_CONFIG). +( cd /app && python -m roboco.llm.providers.opencode_config ) # --- SDK server bring-up (Claude-parity) ---------------------------------- # The flow/do MCP servers POST /verb/attempted here for the per-verb circuit diff --git a/roboco/runtime/orchestrator.py b/roboco/runtime/orchestrator.py index 847290c7..e8d35f82 100644 --- a/roboco/runtime/orchestrator.py +++ b/roboco/runtime/orchestrator.py @@ -5198,6 +5198,11 @@ Start by: """ if pr.get("number") is None: return False + # The reviewer reviews PRs the org did NOT author. Skip PRs opened by the + # repo-owner account: a self-review can't post REQUEST_CHANGES (GitHub + # 422), and re-reviewing the org's own in-flight PRs every poll is noise. + if pr.get("author_is_owner"): + return False if self._is_external_pr(pr): if not settings.external_pr_enabled or not self._pr_author_allowed( pr, allowlist diff --git a/roboco/services/git.py b/roboco/services/git.py index bacf5cdc..b10c1ad6 100644 --- a/roboco/services/git.py +++ b/roboco/services/git.py @@ -1630,6 +1630,8 @@ class GitService(BaseService): head = pr.get("head") or {} head_repo = head.get("repo") or {} head_full = head_repo.get("full_name") + login = (pr.get("user") or {}).get("login") + base_owner = (base_full or "").split("/")[0] return { "number": pr.get("number"), "url": pr.get("html_url") or "", @@ -1637,7 +1639,14 @@ class GitService(BaseService): "head_ref": head.get("ref"), "head_sha": head.get("sha"), "is_fork": bool(head_full and head_full != base_full), - "user_login": (pr.get("user") or {}).get("login"), + "user_login": login, + # The reviewer reviews PRs the org did NOT author. A PR opened by the + # repo-owner account is a self-review (GitHub 422s REQUEST_CHANGES on + # your own PR) and re-reviewing the org's own in-flight work is noise — + # ingestion skips these. + "author_is_owner": bool( + login and base_owner and login.lower() == base_owner.lower() + ), "author_association": pr.get("author_association"), } diff --git a/tests/unit/runtime/test_external_pr_classify.py b/tests/unit/runtime/test_external_pr_classify.py index 03b9f7cb..1ee71b4a 100644 --- a/tests/unit/runtime/test_external_pr_classify.py +++ b/tests/unit/runtime/test_external_pr_classify.py @@ -196,3 +196,24 @@ async def test_skip_internal_when_disabled(monkeypatch: pytest.MonkeyPatch) -> N ) assert ok is False svc.ingest_external_pr.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_skip_owner_authored_pr(monkeypatch: pytest.MonkeyPatch) -> None: + # The org's own account opened the PR → self-review, never ingest (even with + # both review modes on). The reviewer reviews PRs the org did NOT author. + monkeypatch.setattr(orch_mod.settings, "external_pr_enabled", True) + monkeypatch.setattr(orch_mod.settings, "internal_pr_enabled", True) + svc = _svc() + pr = { + "number": 213, + "is_fork": False, + "author_is_owner": True, + "author_association": "OWNER", + "head_ref": "feat/grok-provider-seam", + } + ok = await _orch()._ingest_pr_if_reviewable( + svc, SimpleNamespace(id=uuid4()), pr, uuid4(), set() + ) + assert ok is False + svc.ingest_external_pr.assert_not_awaited() diff --git a/tests/unit/services/test_git_inbound_pr.py b/tests/unit/services/test_git_inbound_pr.py index 7bddba14..b1845020 100644 --- a/tests/unit/services/test_git_inbound_pr.py +++ b/tests/unit/services/test_git_inbound_pr.py @@ -94,11 +94,31 @@ async def test_list_open_prs_normalizes_and_flags_fork() -> None: "head_sha": "deadbeef", "is_fork": True, "user_login": "corey", + "author_is_owner": False, "author_association": "CONTRIBUTOR", } # Same-repo head → not a fork. assert internal["is_fork"] is False assert internal["author_association"] == "MEMBER" + # A contributor (not the repo owner) is not flagged as the owner. + assert internal["author_is_owner"] is False + + +@pytest.mark.asyncio +async def test_list_open_prs_flags_owner_authored_pr() -> None: + """A PR opened by the repo-owner account is flagged author_is_owner.""" + svc = _service() + payload = [ + # Author login == repo owner ("acme") → the org's own PR. + _pr(number=9, head_full="acme/repo", login="acme", assoc="OWNER"), + ] + client = _client(_resp(200, json_payload=payload)) + with ( + _patch_project(), + patch("roboco.services.git.httpx.AsyncClient", return_value=client), + ): + out = await svc.list_open_prs("roboco") + assert out[0]["author_is_owner"] is True @pytest.mark.asyncio