Merge pull request #82 from ClaydeCode/clayde/issue-81

max-tet · web-flow · commit 9ba2dbcd87e5 · 2026-05-26T15:10:50.000+02:00
Fix #81: treat assigned PRs as first-class work items
diff --git a/src/clayde/github.py b/src/clayde/github.py
@@ -57,14 +57,24 @@ def get_default_branch(g: Github, owner: str, repo: str) -> str:
 
 
 def get_assigned_issues(g: Github) -> list:
-    """Return all open issues assigned to the authenticated user."""
+    """Return all open issues AND PRs assigned to the authenticated user.
+
+    GitHub models PRs as issues — each item that is a PR will have
+    ``html_url`` containing ``/pull/``.  Use ``is_pull_request_item()`` to
+    distinguish them.
+    """
     try:
         return list(g.get_user().get_issues(filter="assigned", state="open"))
     except GithubException as e:
         log.error("Failed to fetch assigned issues: %s", e)
         return []
 
 
+def is_pull_request_item(item) -> bool:
+    """Return True if an item from get_assigned_issues() is a pull request."""
+    return "/pull/" in item.html_url
+
+
 def find_open_pr(g: Github, owner: str, repo: str, branch_name: str) -> str | None:
     """Return the HTML URL of an open PR for the given branch, or None."""
     pulls = list(_get_repo(g, owner, repo).get_pulls(
diff --git a/src/clayde/orchestrator.py b/src/clayde/orchestrator.py
@@ -37,13 +37,14 @@
     get_pr_review_comments,
     get_pr_reviews,
     is_blocked,
+    is_pull_request_item,
     issue_ref,
     parse_issue_url,
     parse_pr_url,
 )
-from clayde.safety import get_new_visible_comments, has_visible_content
+from clayde.safety import filter_pr_reviews, get_new_visible_comments, has_visible_content
 from clayde.state import get_issue_state, load_state, save_state, update_issue_state
-from clayde.tasks import work
+from clayde.tasks import pr_work, work
 from clayde.telemetry import get_tracer, init_tracer
 
 log = logging.getLogger("clayde.orchestrator")
@@ -173,6 +174,95 @@ def _handle_issue(g: Github, issue: Issue, url: str) -> None:
         log.info("[%s] Cycle complete", label)
 
 
+def _handle_standalone_pr(g: Github, url: str) -> None:
+    """Handle an assigned PR that has no parent tracked issue.
+
+    Checks for new review activity from whitelisted users and invokes Claude
+    to address it.  State is keyed by the PR url.
+    """
+    tracer = get_tracer()
+    with tracer.start_as_current_span("clayde.handle_standalone_pr", attributes={"pr.url": url}) as span:
+        owner, repo, pr_number = parse_pr_url(url)
+        ref = issue_ref(owner, repo, pr_number)
+
+        pr_state = get_issue_state(url)
+        in_progress = pr_state.get("in_progress", False)
+        last_seen_at = _parse_timestamp(pr_state.get("last_seen_at"))
+
+        # Check for new review activity from whitelisted users
+        has_new_review_activity = False
+        try:
+            reviews = get_pr_reviews(g, owner, repo, pr_number)
+            review_comments = get_pr_review_comments(g, owner, repo, pr_number)
+            github_username = get_settings().github_username
+            visible_reviews = filter_pr_reviews(reviews, github_username)
+
+            if last_seen_at is not None:
+                new_reviews = [r for r in visible_reviews if r.submitted_at > last_seen_at]
+            else:
+                new_reviews = list(visible_reviews)
+
+            if new_reviews:
+                new_review_ids = {r.id for r in new_reviews}
+                has_inline = any(
+                    rc.pull_request_review_id in new_review_ids
+                    for rc in review_comments
+                )
+                has_bodies = any(r.body and r.body.strip() for r in new_reviews)
+                if has_inline or has_bodies:
+                    has_new_review_activity = True
+                else:
+                    # Pure approval with no comments — update timestamp only
+                    log.info("[%s] Pure PR approval — updating last_seen_at", ref)
+                    update_issue_state(url, {"last_seen_at": _now_utc()})
+                    span.set_attribute("pr.skip_reason", "pure_approval")
+                    return
+        except Exception as e:
+            log.warning("[%s] Failed to check PR reviews: %s", ref, e)
+
+        should_invoke = in_progress or has_new_review_activity
+
+        if not should_invoke:
+            log.info("[%s] No new review activity — skipping", ref)
+            span.set_attribute("pr.skip_reason", "no_new_activity")
+            return
+
+        # Mark in_progress before invoking Claude
+        update_issue_state(url, {"in_progress": True, "owner": owner, "repo": repo, "number": pr_number})
+
+        log.info("[%s] New review activity — invoking PR work task", ref)
+        try:
+            pr_work.run(url)
+        except (UsageLimitError, InvocationTimeoutError) as e:
+            log.warning("[%s] Usage/timeout limit — will retry next cycle: %s", ref, e)
+            span.set_attribute("pr.status", "retry")
+            return
+        except Exception as e:
+            log.error("[%s] ERROR in PR work task: %s", ref, e)
+            span.set_status(StatusCode.ERROR, str(e))
+            span.record_exception(e)
+            update_issue_state(url, {"in_progress": False})
+            return
+
+        update_issue_state(url, {"in_progress": False, "last_seen_at": _now_utc()})
+        span.set_attribute("pr.status", "completed")
+        log.info("[%s] PR cycle complete", ref)
+
+
+def _is_pr_tracked_as_issue(pr_url: str, issues_state: dict) -> bool:
+    """Return True if pr_url is already tracked as the child PR of a known issue.
+
+    When Clayde opens a PR while working on an issue, the PR URL is stored
+    under the *issue* state entry as ``pr_url``.  In that case the issue
+    handler owns review activity for the PR, so the standalone-PR handler
+    should skip it.
+    """
+    return any(
+        ist.get("pr_url") == pr_url
+        for ist in issues_state.values()
+    )
+
+
 def _prune_closed_issues(g: Github, issues_state: dict) -> None:
     """Remove closed issues from state to prevent stale entries accumulating."""
     to_prune = []
@@ -244,17 +334,26 @@ def main():
         issues_state = load_state().get("issues", {})
 
         if not assigned:
-            log.info("No assigned issues. Going back to sleep.")
+            log.info("No assigned work items. Going back to sleep.")
             provider.force_flush()
             return
 
-        processed = 0
-        for issue in assigned:
-            url = issue.html_url
-            processed += 1
-            _handle_issue(g, issue, url)
-
-        tick_span.set_attribute("issues.processed", processed)
+        issues_count = 0
+        prs_count = 0
+        for item in assigned:
+            url = item.html_url
+            if is_pull_request_item(item):
+                # Only handle PRs that are NOT already tracked as children of a
+                # known issue (those are handled via _handle_issue).
+                if not _is_pr_tracked_as_issue(url, issues_state):
+                    prs_count += 1
+                    _handle_standalone_pr(g, url)
+            else:
+                issues_count += 1
+                _handle_issue(g, item, url)
+
+        tick_span.set_attribute("issues.processed", issues_count)
+        tick_span.set_attribute("prs.processed", prs_count)
 
     provider.force_flush()
 
diff --git a/src/clayde/prompts/pr_work.j2 b/src/clayde/prompts/pr_work.j2
@@ -0,0 +1,34 @@
+You are Clayde, an autonomous software agent. You have been assigned to pull request #{{ number }}.
+
+PR: {{ owner }}/{{ repo }}#{{ number }}: {{ title }}
+PR URL: {{ pr_url }}
+PR BRANCH: {{ branch_name }}
+
+PR DESCRIPTION:
+{{ body }}
+
+PR REVIEWS:
+{{ review_text }}
+
+REPOSITORY ON DISK: {{ repo_path }}
+
+---
+
+Address the review feedback on this pull request. The PR is already open on branch `{{ branch_name }}`.
+
+Steps:
+1. Ensure you are on branch `{{ branch_name }}` (check out or pull it as needed).
+2. Read the review comments carefully and implement all requested changes.
+3. Run the test suite to confirm nothing is broken.
+4. Commit your changes with a clear message.
+5. Push: git push origin {{ branch_name }}
+
+Do NOT open a new pull request — the existing PR {{ pr_url }} will automatically update when you push to `{{ branch_name }}`.
+
+After completing your work, provide a short summary of what you did.
+
+IMPORTANT: Your final response must be ONLY a raw JSON object — nothing else.
+Do not include any text before or after the JSON. Do not wrap it in markdown code fences.
+Your entire response must be parseable by json.loads().
+
+{"summary": "<short summary of actions taken>"}
diff --git a/src/clayde/safety.py b/src/clayde/safety.py
@@ -57,6 +57,19 @@ def get_new_visible_comments(comments: list, last_seen_at: datetime | None) -> l
     ]
 
 
+def filter_pr_reviews(reviews: list, github_username: str) -> list:
+    """Return only PR reviews from whitelisted users, excluding the bot's own.
+
+    A review is visible if the reviewer is in the whitelist and is not the
+    authenticated bot account.
+    """
+    whitelist = get_settings().whitelisted_users_list
+    return [
+        r for r in reviews
+        if r.user.login in whitelist and r.user.login != github_username
+    ]
+
+
 def has_visible_content(issue, comments: list) -> bool:
     """Return True if there is any visible content (issue body or comments).
 
diff --git a/src/clayde/tasks/pr_work.py b/src/clayde/tasks/pr_work.py
@@ -0,0 +1,125 @@
+"""PR work task — address review comments on a standalone assigned PR.
+
+A "standalone PR" is a pull request assigned to Clayde that has no originating
+issue in the plan → implement → PR lifecycle.  State is keyed by the PR URL
+rather than an issue URL.
+"""
+
+import logging
+
+from clayde.claude import format_cost_line, invoke_claude
+from clayde.config import get_github_client, get_settings
+from clayde.git import ensure_repo
+from clayde.github import (
+    get_default_branch,
+    get_pr_review_comments,
+    get_pr_reviews,
+    issue_ref,
+    parse_pr_url,
+    post_comment,
+)
+from clayde.prompts import render_template
+from clayde.responses import WorkResponse, parse_response
+from clayde.safety import filter_pr_reviews
+from clayde.state import get_issue_state, update_issue_state
+from clayde.telemetry import get_tracer
+
+log = logging.getLogger("clayde.tasks.pr_work")
+
+
+def _format_reviews(reviews: list, review_comments: list) -> str:
+    """Format PR reviews and inline comments into text for the prompt."""
+    parts = []
+    for review in reviews:
+        header = f"Review by @{review.user.login} (state: {review.state}):"
+        inline = [rc for rc in review_comments if rc.pull_request_review_id == review.id]
+        has_body = review.body and review.body.strip()
+        if has_body or inline:
+            parts.append(f"{header}\n{review.body}" if has_body else header)
+        for rc in inline:
+            file_info = f"  File: {rc.path}"
+            if hasattr(rc, "line") and rc.line:
+                file_info += f", line {rc.line}"
+            parts.append(f"{file_info}\n  {rc.body}")
+    return "\n---\n".join(parts) or "(none)"
+
+
+def run(pr_url: str) -> None:
+    """Fetch PR context and invoke Claude to address review comments.
+
+    State is keyed by *pr_url* (not an issue URL).  Raises UsageLimitError or
+    InvocationTimeoutError on rate/timeout limits so the orchestrator can
+    leave in_progress=True for automatic retry.
+    """
+    tracer = get_tracer()
+    with tracer.start_as_current_span("clayde.task.pr_work") as span:
+        g = get_github_client()
+        owner, repo, pr_number = parse_pr_url(pr_url)
+        ref = issue_ref(owner, repo, pr_number)
+        span.set_attribute("pr.number", pr_number)
+        span.set_attribute("pr.owner", owner)
+        span.set_attribute("pr.repo", repo)
+
+        repo_obj = g.get_repo(f"{owner}/{repo}")
+        pr = repo_obj.get_pull(pr_number)
+        title = pr.title
+        body = pr.body or "(empty)"
+        branch_name = pr.head.ref
+        default_branch = get_default_branch(g, owner, repo)
+
+        # Fetch and whitelist-filter reviews
+        settings = get_settings()
+        github_username = settings.github_username
+        reviews = get_pr_reviews(g, owner, repo, pr_number)
+        review_comments = get_pr_review_comments(g, owner, repo, pr_number)
+        visible_reviews = filter_pr_reviews(reviews, github_username)
+        review_text = _format_reviews(visible_reviews, review_comments)
+
+        # Persist metadata before invoking Claude
+        update_issue_state(pr_url, {
+            "owner": owner,
+            "repo": repo,
+            "number": pr_number,
+            "pr_title": title,
+            "branch_name": branch_name,
+            "is_standalone_pr": True,
+        })
+
+        repo_path = ensure_repo(owner, repo, default_branch)
+
+        prompt = render_template(
+            "pr_work.j2",
+            number=pr_number,
+            title=title,
+            owner=owner,
+            repo=repo,
+            body=body,
+            review_text=review_text,
+            repo_path=repo_path,
+            branch_name=branch_name,
+            pr_url=pr_url,
+            default_branch=default_branch,
+        )
+
+        log.info("[%s: %s] Invoking Claude for PR review", ref, title)
+
+        # UsageLimitError/InvocationTimeoutError propagate to the orchestrator
+        result = invoke_claude(prompt, repo_path)
+
+        span.set_attribute("pr_work.output_length", len(result.output or ""))
+
+        # Parse summary (best-effort; fall back to raw output snippet)
+        summary = None
+        try:
+            parsed = parse_response(result.output, WorkResponse)
+            summary = parsed.summary
+        except ValueError:
+            log.warning("[%s: %s] Failed to parse PR work response JSON — using raw output",
+                        ref, title)
+            summary = (result.output or "").strip()[:500] or None
+
+        if summary:
+            post_comment(g, owner, repo, pr_number,
+                         f"{summary}{format_cost_line(result.cost_eur)}")
+
+        log.info("[%s: %s] PR work complete", ref, title)
diff --git a/tests/test_github.py b/tests/test_github.py
@@ -20,6 +20,7 @@
     get_pr_review_comments,
     get_pr_reviews,
     is_blocked,
+    is_pull_request_item,
     parse_issue_url,
     parse_pr_url,
     post_comment,
@@ -54,6 +55,23 @@ def test_invalid_url_raises(self):
             parse_pr_url("https://github.com/alice/repo/issues/1")
 
 
+class TestIsPullRequestItem:
+    def test_returns_true_for_pr_url(self):
+        item = MagicMock()
+        item.html_url = "https://github.com/o/r/pull/80"
+        assert is_pull_request_item(item) is True
+
+    def test_returns_false_for_issue_url(self):
+        item = MagicMock()
+        item.html_url = "https://github.com/o/r/issues/1"
+        assert is_pull_request_item(item) is False
+
+    def test_returns_false_for_arbitrary_url(self):
+        item = MagicMock()
+        item.html_url = "https://github.com/o/r"
+        assert is_pull_request_item(item) is False
+
+
 class TestFetchIssue:
     def test_calls_correct_api(self):
         g = MagicMock()
diff --git a/tests/test_orchestrator.py b/tests/test_orchestrator.py
diff --git a/tests/test_safety.py b/tests/test_safety.py
diff --git a/tests/test_tasks_pr_work.py b/tests/test_tasks_pr_work.py
