feat(robomp): resume needs-info issues (#2728)
This commit is contained in:
@@ -23,6 +23,7 @@ IssueState = Literal[
|
||||
"opened",
|
||||
"merged",
|
||||
"closed",
|
||||
"needs_info",
|
||||
"abandoned",
|
||||
]
|
||||
|
||||
|
||||
@@ -79,6 +79,7 @@ class GitHubBackend(Protocol):
|
||||
) -> None: ...
|
||||
|
||||
async def add_issue_labels(self, repo: str, number: int, labels: list[str]) -> tuple[str, ...]: ...
|
||||
async def remove_issue_label(self, repo: str, number: int, label: str) -> None: ...
|
||||
|
||||
async def submit_pr_review(
|
||||
self,
|
||||
|
||||
@@ -7,6 +7,7 @@ import time
|
||||
from collections.abc import Mapping
|
||||
from dataclasses import dataclass
|
||||
from typing import Any
|
||||
from urllib.parse import quote
|
||||
|
||||
import httpx
|
||||
|
||||
@@ -446,6 +447,16 @@ class GitHubClient:
|
||||
)
|
||||
return tuple(str(lbl["name"]) if isinstance(lbl, dict) else str(lbl) for lbl in (data or []))
|
||||
|
||||
async def remove_issue_label(self, repo: str, number: int, label: str) -> None:
|
||||
"""Remove one label from an issue (or PR)."""
|
||||
if not label:
|
||||
return
|
||||
encoded = quote(label, safe="")
|
||||
await self.request(
|
||||
"DELETE",
|
||||
f"/repos/{repo}/issues/{number}/labels/{encoded}",
|
||||
)
|
||||
|
||||
async def submit_pr_review(
|
||||
self,
|
||||
*,
|
||||
|
||||
@@ -23,7 +23,7 @@ from omp_rpc import HostTool, HostToolContext, RpcCommandError, host_tool
|
||||
|
||||
from robomp import persona
|
||||
from robomp.config import Settings
|
||||
from robomp.db import Database, issue_key
|
||||
from robomp.db import Database, IssueState, issue_key
|
||||
from robomp.git_ops import GitCommandError, HeadDriftError
|
||||
from robomp.github_backend import GitHubBackend
|
||||
from robomp.github_client import GitHubError, IssueInfo, PullRequestFileInfo, RepoInfo
|
||||
@@ -49,6 +49,7 @@ _REPO_COMMAND_SCRUBBED_ENV_KEYS: tuple[str, ...] = (
|
||||
"ROBOMP_REPLAY_TOKEN",
|
||||
"ROBOMP_GH_PROXY_HMAC_KEY",
|
||||
)
|
||||
_NEEDS_INFO_LABEL = "needs-info"
|
||||
_AGENT_HOME = Path("/srv/agent-home")
|
||||
_PRE_PR_FIX_TIMEOUT_SECONDS = 600.0
|
||||
_PRE_PR_CHECK_TIMEOUT_SECONDS = 600.0
|
||||
@@ -137,6 +138,43 @@ def _run_coro(loop: asyncio.AbstractEventLoop, coro: Any) -> Any:
|
||||
return future.result()
|
||||
|
||||
|
||||
def _issue_needs_info(bindings: ToolBindings) -> bool:
|
||||
row = bindings.db.get_issue(bindings.issue_key)
|
||||
return row is not None and row.state == "needs_info"
|
||||
|
||||
|
||||
def _optional_label_error(exc: Exception) -> str:
|
||||
return f"{type(exc).__name__}: {exc}"
|
||||
|
||||
|
||||
def _remove_needs_info_label(bindings: ToolBindings) -> bool:
|
||||
try:
|
||||
_run_coro(
|
||||
bindings.loop,
|
||||
bindings.github.remove_issue_label(bindings.repo.full_name, bindings.issue.number, _NEEDS_INFO_LABEL),
|
||||
)
|
||||
except GitHubError as exc:
|
||||
if exc.status == 404:
|
||||
return True
|
||||
log.warning("needs-info label cleanup failed", extra={"issue": bindings.issue_key, "err": str(exc)})
|
||||
return False
|
||||
except Exception as exc: # noqa: BLE001 - best-effort optional label cleanup
|
||||
log.warning(
|
||||
"needs-info label cleanup failed",
|
||||
extra={"issue": bindings.issue_key, "err": _optional_label_error(exc)},
|
||||
)
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
def _advance_needs_info(bindings: ToolBindings, state: IssueState) -> bool:
|
||||
if not _issue_needs_info(bindings):
|
||||
return False
|
||||
label_cleared = _remove_needs_info_label(bindings)
|
||||
bindings.db.set_issue_state(bindings.issue_key, state)
|
||||
return label_cleared
|
||||
|
||||
|
||||
def _audit(
|
||||
bindings: ToolBindings, name: str, args: Mapping[str, Any], result: Any | None = None, error: str | None = None
|
||||
) -> None:
|
||||
@@ -405,17 +443,14 @@ def _run_pre_publish_bun_check(
|
||||
_raise_command(msg)
|
||||
|
||||
|
||||
_AUTOCLOSE_INELIGIBLE_STATES: frozenset[str] = frozenset({"closed", "merged", "abandoned"})
|
||||
_AUTOCLOSE_INELIGIBLE_STATES: frozenset[str] = frozenset({"closed", "merged", "needs_info", "abandoned"})
|
||||
|
||||
|
||||
def _should_schedule_autoclose(bindings: ToolBindings, target_number: int) -> float | None:
|
||||
"""Return the configured close window (hours) when this comment should
|
||||
schedule an auto-close; ``None`` otherwise.
|
||||
|
||||
Conditions: feature enabled in `Settings`, the comment lands on the
|
||||
originating issue (not a different number, not a PR thread), the issue is
|
||||
classified as `question`, and the issue is not already in a terminal
|
||||
state (closed/merged/abandoned).
|
||||
schedule the question auto-close job: feature enabled, same issue,
|
||||
classified as `question`, and the issue is not already in a terminal or
|
||||
waiting-for-reporter state.
|
||||
"""
|
||||
settings = bindings.settings
|
||||
if settings is None or not settings.question_autoclose_enabled:
|
||||
@@ -697,6 +732,7 @@ def _build_open_pr(bindings: ToolBindings) -> HostTool[Any, Any]:
|
||||
# Make sure the branch is pushed (idempotent) using the same preflight as gh_push_branch.
|
||||
_guarded_push_branch(bindings, args, "gh_open_pr", bindings.workspace.branch)
|
||||
base = args.get("base") or bindings.repo.default_branch
|
||||
was_needs_info = _issue_needs_info(bindings)
|
||||
try:
|
||||
pr = _run_coro(
|
||||
bindings.loop,
|
||||
@@ -714,6 +750,7 @@ def _build_open_pr(bindings: ToolBindings) -> HostTool[Any, Any]:
|
||||
_raise_command(f"GitHub rejected PR: {exc.status} {exc.message}")
|
||||
bindings.db.set_issue_pr(bindings.issue_key, pr.number)
|
||||
bindings.db.set_issue_state(bindings.issue_key, "opened")
|
||||
needs_info_label_cleared = _remove_needs_info_label(bindings) if was_needs_info else False
|
||||
artifact = bindings.workspace.artifacts_dir / "pr.json"
|
||||
artifact.write_text(
|
||||
json.dumps(
|
||||
@@ -728,7 +765,10 @@ def _build_open_pr(bindings: ToolBindings) -> HostTool[Any, Any]:
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
_audit(bindings, "gh_open_pr", args, result={"pr_number": pr.number, "url": pr.html_url})
|
||||
result: dict[str, Any] = {"pr_number": pr.number, "url": pr.html_url}
|
||||
if needs_info_label_cleared:
|
||||
result["cleared_needs_info"] = True
|
||||
_audit(bindings, "gh_open_pr", args, result=result)
|
||||
return f"opened #{pr.number}: {pr.html_url}"
|
||||
|
||||
return host_tool(
|
||||
@@ -842,7 +882,10 @@ def _build_repro_record(bindings: ToolBindings) -> HostTool[Any, Any]:
|
||||
if _slot_permissions_active(bindings.slot_uid):
|
||||
assert bindings.slot_uid is not None
|
||||
os.chown(target, bindings.slot_uid, bindings.slot_uid)
|
||||
_audit(bindings, "repro_record", args, result={"path": str(target.relative_to(bindings.workspace.root))})
|
||||
result: dict[str, Any] = {"path": str(target.relative_to(bindings.workspace.root))}
|
||||
if _advance_needs_info(bindings, "reproducing"):
|
||||
result["cleared_needs_info"] = True
|
||||
_audit(bindings, "repro_record", args, result=result)
|
||||
return "recorded"
|
||||
|
||||
return host_tool(
|
||||
@@ -888,9 +931,26 @@ def _build_mark_unable(bindings: ToolBindings) -> HostTool[Any, Any]:
|
||||
except GitHubError as exc:
|
||||
_audit(bindings, "mark_unable_to_reproduce", args, error=str(exc))
|
||||
_raise_command(f"GitHub rejected comment: {exc.status} {exc.message}")
|
||||
bindings.db.set_issue_state(bindings.issue_key, "abandoned")
|
||||
_audit(bindings, "mark_unable_to_reproduce", args, result={"comment_id": comment.id})
|
||||
return f"posted abandonment comment id={comment.id}"
|
||||
result: dict[str, Any] = {"comment_id": comment.id, "state": "needs_info"}
|
||||
try:
|
||||
labels = _run_coro(
|
||||
bindings.loop,
|
||||
bindings.github.add_issue_labels(bindings.repo.full_name, bindings.issue.number, [_NEEDS_INFO_LABEL]),
|
||||
)
|
||||
result["labels"] = list(labels)
|
||||
except GitHubError as exc:
|
||||
# Some repos have not created the optional status label yet. The
|
||||
# durable behavior is the non-terminal sqlite state plus the visible
|
||||
# info-request comment, so label setup must not block resumption.
|
||||
log.warning("needs-info label failed", extra={"issue": bindings.issue_key, "err": str(exc)})
|
||||
result["label_error"] = f"{exc.status} {exc.message}"
|
||||
except Exception as exc: # noqa: BLE001 - best-effort optional label setup
|
||||
error = _optional_label_error(exc)
|
||||
log.warning("needs-info label failed", extra={"issue": bindings.issue_key, "err": error})
|
||||
result["label_error"] = error
|
||||
bindings.db.set_issue_state(bindings.issue_key, "needs_info")
|
||||
_audit(bindings, "mark_unable_to_reproduce", args, result=result)
|
||||
return f"posted needs-info comment id={comment.id}"
|
||||
|
||||
return host_tool(
|
||||
name="mark_unable_to_reproduce",
|
||||
|
||||
@@ -3,12 +3,12 @@ You ended your turn before finishing.
|
||||
Issue: {{repo.full_name}}#{{issue.number}} — {{issue.title}}
|
||||
Branch: `{{workspace.branch}}`
|
||||
|
||||
You classified this issue and reproduced the bug, but did NOT reach a terminal action. Acceptable terminal actions for a `bug` / `documentation` issue are exactly one of:
|
||||
You classified this issue and reproduced the bug, but did NOT reach a turn-ending action. Acceptable turn-ending actions for a `bug` / `documentation` issue are exactly one of:
|
||||
|
||||
1. `gh_push_branch` + `gh_open_pr` — you committed the fix, pushed the branch, and opened a PR.
|
||||
2. `mark_unable_to_reproduce` — you genuinely cannot reproduce or fix and need maintainer input.
|
||||
2. `mark_unable_to_reproduce` — you genuinely cannot reproduce after a real attempt and need reporter-provided reproduction details.
|
||||
3. `abort_task` — unrecoverable environment failure.
|
||||
|
||||
Review your TodoList and the prior tool calls, then continue from where you stopped. Do NOT re-classify, do NOT re-post the same preamble comment. If your fix is already drafted in the worktree, commit, push, and open the PR now. If you have not yet edited any source files, do the fix and continue through to PR.
|
||||
|
||||
You MUST end this turn by calling one of the three terminal tools listed above.
|
||||
You MUST end this turn by calling one of the three turn-ending tools listed above.
|
||||
|
||||
@@ -61,10 +61,10 @@ description = "Persist a reproduction transcript (command, output, exit code) fo
|
||||
reproduced = "True when the recorded run demonstrates the bug."
|
||||
|
||||
[mark_unable_to_reproduce]
|
||||
description = "Close the loop without a PR: comment with diagnosis + info request, mark issue abandoned."
|
||||
description = "Ask the reporter for missing reproduction details, mark the issue `needs_info`, and keep the session resumable for the next reply."
|
||||
|
||||
[abort_task]
|
||||
description = "Irrecoverably abandon this task WITHOUT posting any visible message. Use ONLY for orchestrator/environment defects you cannot work around (broken filesystem permissions, missing system tools, corrupted git metadata, harness bugs). NEVER for normal workflow problems — failed builds, missing repro info, unclear requests use `gh_post_comment` or `mark_unable_to_reproduce` instead. `reason` is audit-only and NEVER shown to the reporter."
|
||||
description = "Irrecoverably abandon this task WITHOUT posting any visible message. Use ONLY for orchestrator/environment defects you cannot work around (broken filesystem permissions, missing system tools, corrupted git metadata, harness bugs). NEVER for normal workflow problems: failed builds and unclear maintainer requests use `gh_post_comment`; missing reporter reproduction details use `mark_unable_to_reproduce`. `reason` is audit-only and NEVER shown to the reporter."
|
||||
|
||||
[abort_task.parameters]
|
||||
reason = "Internal diagnosis for the operator. Concrete, specific, blameless. NEVER shown to the reporter."
|
||||
|
||||
@@ -28,4 +28,4 @@ the classification calls for code. Drive the todo list to completion:
|
||||
- `invalid` / `duplicate` → one brief comment, then stop.
|
||||
|
||||
3. If `bug` and you cannot reproduce after a real attempt, call
|
||||
`mark_unable_to_reproduce`. You NEVER guess at fixes.
|
||||
`mark_unable_to_reproduce` with the exact reporter details needed. You NEVER guess at fixes.
|
||||
|
||||
@@ -46,7 +46,7 @@ NEVER apply `provider` or `platform` speculatively. They REQUIRE explicit eviden
|
||||
9. **Publish.** Call `gh_push_branch`, then `gh_open_pr`. Both deterministically run `bun run fix` (auto-committing as `style: bun run fix`) then `bun check` before touching the remote. The same gate runs on every follow-up `gh_push_branch`. The tools also refuse dirty trees and commit-author mismatches.
|
||||
- `bun check` failed? Fix at the source, commit, call again.
|
||||
- **Escape hatch — `skip_checks=true`.** ONLY for breakage you have VERIFIED is pre-existing on the default branch. Verify by running the same command against the same paths on a clean checkout of the default branch and confirming the identical failure. NEVER use it to bypass a failure your diff introduced, and NEVER for transient or unclear failures. Document the bypass in the PR's `## Verification` section, one sentence: ``bun check` fails on `main` for unrelated reason X; skipped pre-publish gate.`
|
||||
- **NEVER tamper with git internals.** No editing `.git`/`gitdir:` pointers, no chown/chmod on worktree files, no `safe.directory` overrides, no pointing HEAD at a fabricated commit. Push refused for reasons you cannot resolve? Ask the maintainer via `gh_post_comment`, or use `mark_unable_to_reproduce`. Environmental/orchestrator defect that's not the reporter's problem (broken permissions, corrupted git metadata, missing tools)? Call `abort_task` with the diagnosis — silent abandonment, no comment leaked to the reporter. NEVER improvise.
|
||||
- **NEVER tamper with git internals.** No editing `.git`/`gitdir:` pointers, no chown/chmod on worktree files, no `safe.directory` overrides, no pointing HEAD at a fabricated commit. Push refused for reasons you cannot resolve? Ask the maintainer via `gh_post_comment`. Environmental/orchestrator defect that's not the reporter's problem (broken permissions, corrupted git metadata, missing tools)? Call `abort_task` with the diagnosis — silent abandonment, no comment leaked to the reporter. NEVER improvise.
|
||||
- **Two-strikes rule.** Two consecutive `gh_push_branch` rejections with the same error is a workflow bug. Fix the cause, use `skip_checks=true` with justification, or escalate via `gh_post_comment`. NEVER loop.
|
||||
10. **Link.** After the PR opens, one final `gh_post_comment` linking it.
|
||||
|
||||
|
||||
@@ -5,3 +5,5 @@
|
||||
## Information needed
|
||||
|
||||
{{info_needed}}
|
||||
|
||||
I'll keep this issue waiting on reporter details and resume from this context when the requested information arrives.
|
||||
|
||||
@@ -509,6 +509,19 @@ def create_proxy_app(settings: Settings) -> FastAPI:
|
||||
return _gh_error_response(exc)
|
||||
return JSONResponse({"labels": list(applied)})
|
||||
|
||||
@app.post("/gh/v1/remove_issue_label")
|
||||
async def remove_issue_label(request: Request) -> JSONResponse:
|
||||
data = await _json_body(request)
|
||||
repo = _require_str(data.get("repo"), "repo")
|
||||
number = _require_int(data.get("number"), "number")
|
||||
label = _require_str(data.get("label"), "label")
|
||||
github: GitHubClient = request.app.state.github
|
||||
try:
|
||||
await github.remove_issue_label(repo, number, label)
|
||||
except GitHubError as exc:
|
||||
return _gh_error_response(exc)
|
||||
return JSONResponse({"ok": True})
|
||||
|
||||
@app.post("/gh/v1/submit_pr_review")
|
||||
async def submit_pr_review(request: Request) -> JSONResponse:
|
||||
data = await _json_body(request)
|
||||
|
||||
@@ -282,6 +282,15 @@ class GitHubProxyClient:
|
||||
)
|
||||
return tuple(str(lbl) for lbl in (data.get("labels") if isinstance(data, dict) else None) or [])
|
||||
|
||||
async def remove_issue_label(self, repo: str, number: int, label: str) -> None:
|
||||
if not label:
|
||||
return
|
||||
await self._request(
|
||||
"POST",
|
||||
"/gh/v1/remove_issue_label",
|
||||
json_body={"repo": repo, "number": number, "label": label},
|
||||
)
|
||||
|
||||
async def submit_pr_review(
|
||||
self,
|
||||
*,
|
||||
|
||||
Reference in New Issue
Block a user