mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(grok): unbreak workspace-cwd agents, free trapped agents, stop self-PR review
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).
This commit is contained in:
@@ -81,12 +81,21 @@ function bareVerb(tool) {
|
|||||||
return 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
|
// Named export (opencode's plugin convention) + baked into the plugin
|
||||||
// auto-discovery dir (~/.config/opencode/plugin/) at image build — the simplest
|
// auto-discovery dir (~/.config/opencode/plugin/) at image build — the simplest
|
||||||
// registration route (no config `plugin:` path needed).
|
// registration route (no config `plugin:` path needed).
|
||||||
export const RobocoBudgetFeed = async () => {
|
export const RobocoBudgetFeed = async () => {
|
||||||
return {
|
return {
|
||||||
"tool.execute.before": async (input) => {
|
"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);
|
const status = await sdk("GET", "/budget/status", null);
|
||||||
if (!status) {
|
if (!status) {
|
||||||
// One-shot delivery agents MUST have the in-container SDK budget server
|
// One-shot delivery agents MUST have the in-container SDK budget server
|
||||||
|
|||||||
@@ -15,7 +15,12 @@ SDK_URL="http://localhost:${SDK_PORT}"
|
|||||||
|
|
||||||
# Generate opencode.json (provider + model + MCP gateway + permissions +
|
# Generate opencode.json (provider + model + MCP gateway + permissions +
|
||||||
# instructions). Writes to opencode's global config dir by default.
|
# 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) ----------------------------------
|
# --- SDK server bring-up (Claude-parity) ----------------------------------
|
||||||
# The flow/do MCP servers POST /verb/attempted here for the per-verb circuit
|
# The flow/do MCP servers POST /verb/attempted here for the per-verb circuit
|
||||||
|
|||||||
@@ -5198,6 +5198,11 @@ Start by:
|
|||||||
"""
|
"""
|
||||||
if pr.get("number") is None:
|
if pr.get("number") is None:
|
||||||
return False
|
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 self._is_external_pr(pr):
|
||||||
if not settings.external_pr_enabled or not self._pr_author_allowed(
|
if not settings.external_pr_enabled or not self._pr_author_allowed(
|
||||||
pr, allowlist
|
pr, allowlist
|
||||||
|
|||||||
+10
-1
@@ -1630,6 +1630,8 @@ class GitService(BaseService):
|
|||||||
head = pr.get("head") or {}
|
head = pr.get("head") or {}
|
||||||
head_repo = head.get("repo") or {}
|
head_repo = head.get("repo") or {}
|
||||||
head_full = head_repo.get("full_name")
|
head_full = head_repo.get("full_name")
|
||||||
|
login = (pr.get("user") or {}).get("login")
|
||||||
|
base_owner = (base_full or "").split("/")[0]
|
||||||
return {
|
return {
|
||||||
"number": pr.get("number"),
|
"number": pr.get("number"),
|
||||||
"url": pr.get("html_url") or "",
|
"url": pr.get("html_url") or "",
|
||||||
@@ -1637,7 +1639,14 @@ class GitService(BaseService):
|
|||||||
"head_ref": head.get("ref"),
|
"head_ref": head.get("ref"),
|
||||||
"head_sha": head.get("sha"),
|
"head_sha": head.get("sha"),
|
||||||
"is_fork": bool(head_full and head_full != base_full),
|
"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"),
|
"author_association": pr.get("author_association"),
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -196,3 +196,24 @@ async def test_skip_internal_when_disabled(monkeypatch: pytest.MonkeyPatch) -> N
|
|||||||
)
|
)
|
||||||
assert ok is False
|
assert ok is False
|
||||||
svc.ingest_external_pr.assert_not_awaited()
|
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()
|
||||||
|
|||||||
@@ -94,11 +94,31 @@ async def test_list_open_prs_normalizes_and_flags_fork() -> None:
|
|||||||
"head_sha": "deadbeef",
|
"head_sha": "deadbeef",
|
||||||
"is_fork": True,
|
"is_fork": True,
|
||||||
"user_login": "corey",
|
"user_login": "corey",
|
||||||
|
"author_is_owner": False,
|
||||||
"author_association": "CONTRIBUTOR",
|
"author_association": "CONTRIBUTOR",
|
||||||
}
|
}
|
||||||
# Same-repo head → not a fork.
|
# Same-repo head → not a fork.
|
||||||
assert internal["is_fork"] is False
|
assert internal["is_fork"] is False
|
||||||
assert internal["author_association"] == "MEMBER"
|
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
|
@pytest.mark.asyncio
|
||||||
|
|||||||
Reference in New Issue
Block a user