diff --git a/README.md b/README.md index 0a83392..34b34e5 100644 --- a/README.md +++ b/README.md @@ -12,53 +12,28 @@ Clayde is a persistent autonomous AI software agent that lives on a dedicated VM Clayde is assigned GitHub issues in software repositories. For each issue it: -1. Researches the codebase and writes a **preliminary plan**, posting it as a GitHub comment -2. Waits for human approval (a 👍 reaction) before continuing -3. For **large issues**: writes a **detailed implementation plan** and posts it as another comment, then waits for approval again before touching any code. For **small issues**: skips directly to implementation after preliminary approval. -4. Implements the solution on a new branch, opens a pull request, and posts a summary comment -5. Addresses any review comments left on the PR +1. Checks for new whitelist-visible activity since its last access +2. Invokes Claude with the full issue context — Claude decides what to do next: ask clarifying questions, post a plan, implement the solution, or address review comments +3. Posts a summary comment after each work cycle +4. Opens a pull request (Claude creates the PR directly with a description and, for diffs spanning more than 3 files, a recommended reading order) and assigns the issue author as reviewer +5. Monitors the PR and addresses review comments when they appear -At any point while waiting for approval, new comments on the issue will trigger a plan update — Clayde revises the plan and posts a summary of what changed. - -Clayde runs as a Docker container in a continuous loop (default: every 5 minutes), driven by a state machine persisted in `data/state.json`. +Clayde runs as a Docker container in a continuous loop (default: every 5 minutes). Rather than a rigid state machine, it uses **timestamp-based activity detection**: each issue records the last time it was processed, and only new visible activity since that timestamp triggers a new Claude invocation. --- -## State Machine - -Each issue moves through the following states: - -```mermaid -stateDiagram-v2 - [*] --> preliminary_planning - preliminary_planning --> awaiting_preliminary_approval - awaiting_preliminary_approval --> awaiting_preliminary_approval: new comment → plan updated - awaiting_preliminary_approval --> planning: 👍 approved (large issue) - awaiting_preliminary_approval --> implementing: 👍 approved (small issue) - planning --> awaiting_plan_approval - awaiting_plan_approval --> awaiting_plan_approval: new comment → plan updated - awaiting_plan_approval --> implementing: 👍 approved - implementing --> pr_open - pr_open --> addressing_review: review comments received - addressing_review --> pr_open - pr_open --> done: PR approved - implementing --> failed: error - done --> [*] - failed --> [*] -``` +## How It Works -| State | Description | -|---|---| -| `preliminary_planning` | Claude explores the codebase and writes a short overview with clarifying questions | -| `awaiting_preliminary_approval` | Preliminary plan posted; waiting for 👍 from an approver. New comments trigger a plan update. Small issues skip to `implementing` on approval; large issues proceed to `planning`. | -| `planning` | Claude writes a detailed implementation plan | -| `awaiting_plan_approval` | Full plan posted; waiting for 👍 from an approver. New comments trigger a plan update. | -| `implementing` | Claude implements the solution on a new branch | -| `pr_open` | PR opened; monitoring for review comments | -| `addressing_review` | Claude is addressing PR review comments | -| `done` | PR approved; issue complete | -| `failed` | Error occurred; requires manual reset to retry | -| `interrupted` | Claude hit a usage/rate limit; retried automatically next cycle | +Clayde's loop is event-driven and stateless by design: + +1. **Fetch assigned issues** from GitHub. +2. **For each issue**: check whether there is new whitelist-visible activity (comments or PR reviews) since `last_seen_at`. If the issue has never been seen, or a previous run was interrupted, it is always processed. +3. **Invoke Claude once** with the full context: issue body, all visible comments, and any open PR reviews. Claude decides the next action — no hard phases. +4. **Detect PR**: after each run, check for an open PR on the working branch and persist its URL. +5. **Update `last_seen_at`** to the current time so Clayde's own reply comments don't re-trigger a cycle. +6. **Crash recovery**: `in_progress` is set before invoking Claude and cleared after. If the process crashes mid-run, the next cycle retries automatically. +7. **Pure PR approvals** (no comments) update `last_seen_at` without invoking Claude. +8. **Closed issues** are pruned from state automatically. --- @@ -70,10 +45,7 @@ Clayde uses **content filtering** rather than gatekeeping which issues to work o - If an issue has no visible content at all, it is skipped. - Blocked issues (those with "blocked by #N" or "depends on #N" in the body) are also skipped. -Two approval gates then guard forward progress: - -1. **Preliminary plan approval** — a 👍 from a whitelisted user on the preliminary plan comment is required before the full plan is written. -2. **Plan approval** — a 👍 from a whitelisted user on the full plan comment is required before implementation begins. +There are no hard approval gates — Claude engages with the issue as soon as there is visible content, and the human can steer the conversation by replying in the issue thread. Whitelisted users are configured via `CLAYDE_WHITELISTED_USERS` in `data/config.env`. @@ -82,11 +54,14 @@ Whitelisted users are configured via `CLAYDE_WHITELISTED_USERS` in `data/config. ## Capabilities - **Multi-repo support**: Clones and works on any GitHub repository it has access to -- **Two-phase planning**: Preliminary exploration followed by a detailed plan, each gated by human approval -- **Full issue lifecycle**: Plan → approval → implement → PR, with comments at each stage +- **Event-driven loop**: Only invokes Claude when there is new visible activity — no wasted cycles +- **Natural conversation**: Claude engages directly in the issue comment thread, asking questions and posting plans as needed +- **Full issue lifecycle**: Engage → implement → PR → review, all driven by new activity +- **PR creation by Claude**: Claude writes the PR description and a recommended reading order for larger diffs - **PR review handling**: Reads and addresses reviewer feedback automatically - **Rate-limit resilience**: Detects Claude usage limits and automatically retries -- **Safety gates**: Whitelist + approval checks prevent unauthorized work +- **Crash recovery**: `in_progress` flag ensures interrupted runs are retried next cycle +- **Safety filtering**: Whitelist-based content filtering prevents acting on unauthorized content - **Observability**: OpenTelemetry tracing with JSONL file export - **Dual Claude backend**: Use the Anthropic API (pay-per-token) or the Claude Code CLI (subscription-based) diff --git a/src/clayde/orchestrator.py b/src/clayde/orchestrator.py index 4de49fc..27a9d71 100644 --- a/src/clayde/orchestrator.py +++ b/src/clayde/orchestrator.py @@ -1,11 +1,10 @@ -"""Clayde orchestrator — manages the issue lifecycle state machine. +"""Clayde orchestrator — event-driven issue processing loop. - new → preliminary_planning → awaiting_preliminary_approval - → planning → awaiting_plan_approval → implementing → pr_open → done - ↘ failed - -New comment detection triggers plan updates in awaiting_* states. -PR reviews are handled in pr_open state. +For each assigned issue, Clayde checks whether there has been new +whitelist-visible activity since the last cycle. If so (or if a previous +invocation was interrupted), it calls the unified work task, which lets +Claude decide the next action — ask questions, post a plan, implement, or +address reviews. Entry points: main() — single cycle (one-shot mode, used for testing/debugging) @@ -19,7 +18,7 @@ import subprocess import sys import time -from datetime import datetime +from datetime import datetime, timezone import uvicorn from opentelemetry import trace @@ -28,31 +27,29 @@ from github import Github from github.Issue import Issue -from clayde.claude import is_claude_available +from clayde.claude import InvocationTimeoutError, UsageLimitError, is_claude_available from clayde.config import get_github_client, get_settings, setup_logging from clayde.webhook import JobQueue, create_app, worker_loop from clayde.github import ( fetch_issue, fetch_issue_comments, get_assigned_issues, + get_pr_review_comments, + get_pr_reviews, is_blocked, issue_ref, parse_issue_url, + parse_pr_url, ) -from clayde.safety import get_new_visible_comments, has_visible_content, is_plan_approved -from clayde.state import IssueStatus, get_issue_state, load_state, save_state, update_issue_state -from clayde.tasks import implement, plan, review +from clayde.safety import 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.telemetry import get_tracer, init_tracer log = logging.getLogger("clayde.orchestrator") _shutdown = False -# Backward compatibility: treat old status names as their new equivalents -_STATUS_COMPAT = { - "awaiting_approval": IssueStatus.AWAITING_PLAN_APPROVAL, -} - def _issue_label(issue_state: dict) -> str: """Return 'owner/repo#N: title' for display in log lines.""" @@ -66,161 +63,114 @@ def _issue_label(issue_state: dict) -> str: return f"{ref} (title unknown)" -def _handle_new_issue(g: Github, issue: Issue, url: str) -> None: - """Handle a newly assigned issue — check for visible content and blocked state.""" +def _parse_timestamp(ts: str | None) -> datetime | None: + """Parse an ISO UTC timestamp string into an aware datetime, or return None.""" + if not ts: + return None + try: + return datetime.fromisoformat(ts.replace("Z", "+00:00")) + except (ValueError, AttributeError): + return None + + +def _now_utc() -> str: + """Return the current UTC time as an ISO 8601 string.""" + return datetime.now(timezone.utc).isoformat() + + +def _handle_issue(g: Github, issue: Issue, url: str) -> None: + """Check for new activity and invoke Claude if needed.""" tracer = get_tracer() - with tracer.start_as_current_span("clayde.handle_new_issue", attributes={"issue.url": url}) as span: + with tracer.start_as_current_span("clayde.handle_issue", attributes={"issue.url": url}) as span: owner, repo, number = parse_issue_url(url) - - # Check if issue is blocked by another open issue ref = issue_ref(owner, repo, number) label = f"{ref}: {issue.title}" + + # Skip blocked issues try: if is_blocked(g, owner, repo, number): log.info("[%s] Skipping — blocked by another open issue", label) - span.set_attribute("issue.skipped", True) span.set_attribute("issue.skip_reason", "blocked") return except Exception as e: log.warning("[%s] Failed to check blocked status: %s — proceeding", label, e) - # Check if there is any visible content to work with + # Skip issues with no whitelist-visible content comments = fetch_issue_comments(g, owner, repo, number) if not has_visible_content(issue, comments): log.info("[%s] Skipping — no visible content (all filtered out)", label) - span.set_attribute("issue.skipped", True) span.set_attribute("issue.skip_reason", "no_visible_content") return - log.info("[%s] New issue — starting preliminary plan phase", label) - try: - plan.run_preliminary(url) - span.set_attribute("issue.failed", False) - except Exception as e: - log.error("[%s] ERROR in preliminary plan: %s", label, e) - span.set_status(StatusCode.ERROR, str(e)) - span.record_exception(e) - span.set_attribute("issue.failed", True) - update_issue_state(url, {"status": IssueStatus.FAILED}) - - -def _handle_awaiting_approval(g: Github, url: str, issue_state: dict, *, phase: str) -> None: - """Handle awaiting_*_approval states — check for 👍 or new comments. - - Args: - phase: Either "preliminary" or "thorough". - """ - comment_id_key = "preliminary_comment_id" if phase == "preliminary" else "plan_comment_id" - update_phase = phase - if phase == "preliminary": - # Small issues skip thorough planning and go straight to implementation. - size = issue_state.get("size", "large") - if size == "small": - next_task = implement.run - next_task_label = "implement" - else: - next_task = plan.run_thorough - next_task_label = "thorough_plan" - else: - next_task = implement.run - next_task_label = "implement" + issue_state = get_issue_state(url) + in_progress = issue_state.get("in_progress", False) + last_seen_at = _parse_timestamp(issue_state.get("last_seen_at")) - tracer = get_tracer() - with tracer.start_as_current_span(f"clayde.handle_awaiting_{phase}_approval", attributes={"issue.url": url}) as span: - owner = issue_state["owner"] - repo = issue_state["repo"] - number = issue_state["number"] - comment_id = issue_state.get(comment_id_key) - span.set_attribute("issue.number", number) - - if not comment_id: - log.warning("[%s] No %s — marking as failed", _issue_label(issue_state), comment_id_key) - update_issue_state(url, {"status": IssueStatus.FAILED}) - return + # Check for new visible comments since last cycle + new_comments = get_new_visible_comments(comments, last_seen_at) - # Check for new visible comments first (plan update) - if _has_new_comments(g, owner, repo, number, issue_state): - log.info("[%s] New comments — updating %s plan", _issue_label(issue_state), phase) + # Check for new PR review activity + has_new_review_activity = False + pr_url = issue_state.get("pr_url") + if pr_url and last_seen_at is not None: try: - plan.run_update(url, update_phase) - span.set_attribute("issue.action", "plan_update") + _, _, pr_number = parse_pr_url(pr_url) + 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 + new_reviews = [ + r for r in reviews + if r.submitted_at > last_seen_at + and r.user.login != github_username + ] + 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 (no comments) — update timestamp without invoking Claude + log.info("[%s] Pure PR approval — updating last_seen_at", label) + update_issue_state(url, {"last_seen_at": _now_utc()}) + span.set_attribute("issue.skip_reason", "pure_approval") + return except Exception as e: - log.error("[%s] ERROR updating %s plan: %s", _issue_label(issue_state), phase, e) - span.set_status(StatusCode.ERROR, str(e)) - span.record_exception(e) - return + log.warning("[%s] Failed to check PR reviews: %s", label, e) - # Check for approval (👍 on plan comment) - if is_plan_approved(g, owner, repo, number, comment_id): - log.info("[%s] %s plan approved — running %s", _issue_label(issue_state), phase.capitalize(), next_task_label) - try: - next_task(url) - span.set_attribute("issue.action", next_task_label) - except Exception as e: - log.error("[%s] ERROR in %s: %s", _issue_label(issue_state), next_task_label, e) - span.set_status(StatusCode.ERROR, str(e)) - span.record_exception(e) - update_issue_state(url, {"status": IssueStatus.FAILED}) - return + should_invoke = in_progress or (last_seen_at is None) or bool(new_comments) or has_new_review_activity - log.info("[%s] Awaiting %s approval", _issue_label(issue_state), phase) - span.set_attribute("issue.skipped", True) - span.set_attribute("issue.skip_reason", "not_approved") + if not should_invoke: + log.info("[%s] No new activity — skipping", label) + span.set_attribute("issue.skip_reason", "no_new_activity") + return + # Mark in_progress before invoking Claude so a crash leaves a retry marker + update_issue_state(url, {"in_progress": True}) -def _handle_pr_open(g: Github, url: str, issue_state: dict) -> None: - """Handle pr_open — check for new reviews on the PR.""" - tracer = get_tracer() - with tracer.start_as_current_span("clayde.handle_pr_open", attributes={"issue.url": url}) as span: - span.set_attribute("issue.number", issue_state.get("number", 0)) - log.info("[%s] Checking for PR reviews", _issue_label(issue_state)) + log.info("[%s] New activity — invoking work task", label) try: - review.run(url) - span.set_attribute("issue.failed", False) - except Exception as e: - log.error("[%s] ERROR in review handling: %s", _issue_label(issue_state), e) - span.set_status(StatusCode.ERROR, str(e)) - span.record_exception(e) - span.set_attribute("issue.failed", True) - update_issue_state(url, {"status": IssueStatus.FAILED}) - - -def _handle_interrupted(url: str, issue_state: dict) -> None: - """Handle interrupted issues — retry the interrupted phase.""" - tracer = get_tracer() - phase = issue_state.get("interrupted_phase") - with tracer.start_as_current_span("clayde.handle_interrupted", attributes={"issue.url": url, "issue.interrupted_phase": phase or "unknown"}) as span: - log.info("[%s] Retrying interrupted issue (phase: %s)", _issue_label(issue_state), phase) - - task_map = { - IssueStatus.PRELIMINARY_PLANNING: plan.run_preliminary, - IssueStatus.PLANNING: plan.run_thorough, - IssueStatus.IMPLEMENTING: implement.run, - IssueStatus.ADDRESSING_REVIEW: review.run, - } - task = task_map.get(phase) - if task is None: - log.warning("[%s] Unknown interrupted_phase '%s' — skipping", _issue_label(issue_state), phase) - span.set_attribute("issue.skipped", True) - span.set_attribute("issue.skip_reason", "unknown_phase") + work.run(url) + except (UsageLimitError, InvocationTimeoutError) as e: + log.warning("[%s] Usage/timeout limit — will retry next cycle: %s", label, e) + span.set_attribute("issue.status", "retry") + # in_progress stays True so the next cycle retries automatically return - try: - task(url) - span.set_attribute("issue.failed", False) except Exception as e: - log.error("[%s] ERROR retrying interrupted issue: %s", _issue_label(issue_state), e) + log.error("[%s] ERROR in work task: %s", label, e) span.set_status(StatusCode.ERROR, str(e)) span.record_exception(e) - span.set_attribute("issue.failed", True) - # Stay in interrupted state so we keep retrying. - update_issue_state(url, {"status": IssueStatus.INTERRUPTED}) - + update_issue_state(url, {"in_progress": False}) + return -def _has_new_comments(g: Github, owner: str, repo: str, number: int, issue_state: dict) -> bool: - """Return True if there are new visible comments from non-Clayde users.""" - last_seen = issue_state.get("last_seen_comment_id", 0) - comments = fetch_issue_comments(g, owner, repo, number) - return bool(get_new_visible_comments(comments, last_seen)) + # Successful completion — update last_seen_at to prevent re-triggering on + # Clayde's own comments posted during this run + update_issue_state(url, {"in_progress": False, "last_seen_at": _now_utc()}) + span.set_attribute("issue.status", "completed") + log.info("[%s] Cycle complete", label) def _prune_closed_issues(g: Github, issues_state: dict) -> None: @@ -279,12 +229,11 @@ def main(): tick_span.set_attribute("claude.available", True) g = get_github_client() assigned = get_assigned_issues(g) - state = load_state() - issues_state = state.get("issues", {}) tick_span.set_attribute("issues.assigned_count", len(assigned)) # Prune closed issues from state before any other processing + issues_state = load_state().get("issues", {}) _prune_closed_issues(g, issues_state) # Reload state after pruning @@ -295,55 +244,11 @@ def main(): provider.force_flush() return - # Recover issues stuck in transient states (e.g. from a crash/restart) - _TRANSIENT_STATES = { - IssueStatus.PRELIMINARY_PLANNING, - IssueStatus.PLANNING, - IssueStatus.IMPLEMENTING, - IssueStatus.ADDRESSING_REVIEW, - } - for url, ist in issues_state.items(): - status = ist.get("status") - status = _STATUS_COMPAT.get(status, status) - if status in _TRANSIENT_STATES: - log.warning( - "Recovering stuck %s (was %s → interrupted)", - _issue_label(ist), - status, - ) - update_issue_state( - url, - {"status": IssueStatus.INTERRUPTED, "interrupted_phase": status}, - ) - - # Reload state after recovery mutations - issues_state = load_state().get("issues", {}) - processed = 0 for issue in assigned: url = issue.html_url - issue_state = issues_state.get(url, {}) - status = issue_state.get("status") - - # Backward compatibility - status = _STATUS_COMPAT.get(status, status) - - if status == IssueStatus.DONE: - continue - processed += 1 - if status is None: - _handle_new_issue(g, issue, url) - elif status == IssueStatus.AWAITING_PRELIMINARY_APPROVAL: - _handle_awaiting_approval(g, url, issue_state, phase="preliminary") - elif status == IssueStatus.AWAITING_PLAN_APPROVAL: - _handle_awaiting_approval(g, url, issue_state, phase="thorough") - elif status == IssueStatus.PR_OPEN: - _handle_pr_open(g, url, issue_state) - elif status == IssueStatus.INTERRUPTED: - _handle_interrupted(url, issue_state) - elif status == IssueStatus.FAILED: - log.info("[%s] Skipping failed issue (clear state to retry)", _issue_label(issue_state)) + _handle_issue(g, issue, url) tick_span.set_attribute("issues.processed", processed) diff --git a/src/clayde/prompts.py b/src/clayde/prompts.py index d95f12c..505895b 100644 --- a/src/clayde/prompts.py +++ b/src/clayde/prompts.py @@ -1,4 +1,4 @@ -"""Prompt template utilities and shared comment helpers.""" +"""Prompt template utilities.""" from pathlib import Path @@ -13,20 +13,3 @@ def render_template(name: str, **ctx) -> str: """Render a Jinja2 template from the prompts directory.""" template_src = (PROMPTS_DIR / name).read_text() return _env.from_string(template_src).render(**ctx) - - -def collect_comments_after(comments: list, anchor_id: int) -> str: - """Return formatted text of visible comments posted after anchor_id. - - Comments are formatted as '@login:\\nbody' and joined with '---' separators. - Returns '(none)' when there are no comments after the anchor. - """ - past_anchor = False - parts = [] - for comment in comments: - if comment.id == anchor_id: - past_anchor = True - continue - if past_anchor: - parts.append(f"@{comment.user.login}:\n{comment.body}") - return "\n---\n".join(parts) or "(none)" diff --git a/src/clayde/prompts/address_review.j2 b/src/clayde/prompts/address_review.j2 deleted file mode 100644 index aabdf04..0000000 --- a/src/clayde/prompts/address_review.j2 +++ /dev/null @@ -1,36 +0,0 @@ -A pull request you created has received review comments. Address the feedback -and push updates to the branch. - -ISSUE #{{ number }}: {{ title }} -REPO: {{ owner }}/{{ repo }} - -ISSUE BODY: -{{ body }} - -PR BRANCH: {{ branch_name }} - -REVIEW COMMENTS: -{{ review_text }} - -REPOSITORY ON DISK: {{ repo_path }} - -Steps: -1. Check out the branch: git checkout {{ branch_name }} && git pull origin {{ branch_name }} -2. Read the README (README.md, README.rst, README.txt, or README) and agents.md at the root and in any relevant subdirectories. -3. Read and understand each review comment. -4. Make the requested changes. -5. If any changes affect documented usage, configuration, architecture, or behavior, update the README and agents.md accordingly. -6. Run tests if a test command is discoverable (check CLAUDE.md, README, Makefile, package.json, pyproject.toml, justfile): - - If no test runner is discoverable, skip this step. - - If a test suite exists: ensure all tests pass after your changes (pre-existing failures are exempt). Do NOT commit if your changes introduced new test failures. -7. Commit with a clear message referencing the review, e.g.: "Address review: " -8. Push: git push origin {{ branch_name }} - -After making all changes, provide a short summary of what you changed in response -to each review comment. - -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": ""} diff --git a/src/clayde/prompts/implement.j2 b/src/clayde/prompts/implement.j2 deleted file mode 100644 index 3bf5fc3..0000000 --- a/src/clayde/prompts/implement.j2 +++ /dev/null @@ -1,36 +0,0 @@ -Implement the approved plan for GitHub issue #{{ number }} in repo {{ owner }}/{{ repo }}. - -ISSUE #{{ number }}: {{ title }} - -ISSUE BODY: -{{ body }} - -APPROVED PLAN: -{{ plan_text }} - -ADDITIONAL COMMENTS/INSTRUCTIONS FROM DISCUSSION: -{{ discussion_text }} - -REPOSITORY ON DISK: {{ repo_path }} - -Steps: -1. Before creating a branch, check whether branch `{{ branch_name }}` already exists. If so, resume from the existing state rather than starting fresh. -2. Create or switch to branch: git checkout -b {{ branch_name }} (or git checkout {{ branch_name }} if it exists) -3. Read the README (README.md, README.rst, README.txt, or README) and agents.md at the root and in any relevant subdirectories before writing any code. -4. Implement the plan carefully, following existing code style. -5. After implementing, update the README and agents.md as needed to reflect any changes to usage, configuration, architecture, or behavior. -6. Write or update tests if applicable -7. Run tests if a test command is discoverable (check CLAUDE.md, README, Makefile, package.json, pyproject.toml, justfile): - - If no test runner is discoverable, skip this step. - - If a test suite exists: first note any pre-existing failures (run tests on the base branch before your changes if needed), then ensure all tests pass after your changes (pre-existing failures are exempt). If there are pre-existing failures, you MUST mention them in your PR summary. Do NOT commit if your changes introduced new test failures. -8. Review your own changes for correctness, edge cases, and style and run "just cleanup" if it exists before committing -9. Commit with a clear message, e.g.: "Fix #{{ number }}: " -10. Push: git push origin {{ branch_name }} - -Do NOT create a pull request — it will be created automatically after you finish. - -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": ""} diff --git a/src/clayde/prompts/plan.j2 b/src/clayde/prompts/plan.j2 deleted file mode 100644 index ebfa8de..0000000 --- a/src/clayde/prompts/plan.j2 +++ /dev/null @@ -1,35 +0,0 @@ -Research the following GitHub issue and produce a detailed implementation plan. - -ISSUE #{{ number }}: {{ title }} -REPO: {{ owner }}/{{ repo }} -LABELS: {{ labels }} - -ISSUE BODY: -{{ body }} - -EXISTING COMMENTS: -{{ comments_text }} - -REPOSITORY ON DISK: {{ repo_path }} - -Explore the repository to understand the codebase relevant to this issue. -Pay special attention to agents.md files throughout. -Then produce a plan that includes: -- Summary of what needs to be done and why -- Files to create or modify (with specific locations) -- Implementation approach and key decisions -- Edge cases and risks -- How to verify the implementation is correct - -At the end of your plan, include a branch name suggestion on its own line in -exactly this format: - -**Branch:** `clayde/issue-{{ number }}-` - -where `` is a 1-to-3 word lowercase hyphenated description of the issue -(e.g., `clayde/issue-13-better-branch-naming`). This MUST be present. - -If anything is ambiguous or you need clarification, include your questions -at the end under a "Questions" section (before the branch name line). - -Output ONLY the plan in markdown format. No preamble, no wrapping. diff --git a/src/clayde/prompts/preliminary_plan.j2 b/src/clayde/prompts/preliminary_plan.j2 deleted file mode 100644 index 3b69d57..0000000 --- a/src/clayde/prompts/preliminary_plan.j2 +++ /dev/null @@ -1,40 +0,0 @@ -Review the following GitHub issue and produce a short preliminary plan. - -ISSUE #{{ number }}: {{ title }} -REPO: {{ owner }}/{{ repo }} -LABELS: {{ labels }} - -ISSUE BODY: -{{ body }} - -EXISTING COMMENTS: -{{ comments_text }} - -REPOSITORY ON DISK: {{ repo_path }} - -Before doing anything else, read the following files if they exist in the repository: -- README (any of: README.md, README.rst, README.txt, README) -- agents.md (at the root and any subdirectories relevant to this issue) - -Then explore the repository briefly to understand the codebase relevant to this issue. -Produce a SHORT preliminary plan that includes: -- A brief overview of your approach (a few paragraphs, NOT an exhaustive plan) -- Any clarifying questions you have - -HARD LIMIT: 200 words or fewer. This is just a preliminary overview for -discussion, not the full implementation plan. That comes later after approval -(for large issues). - -Also determine: -- `size`: Classify the issue as `small` (roughly 1–5 files, mechanical or - well-scoped changes, no architectural decisions) or `large` (broader scope, - multiple components, significant design choices). When in doubt, prefer `large`. -- `branch_name`: Choose a branch name in the format - `clayde/issue-{{ number }}-` where `` is a 1-to-3 word lowercase - hyphenated description (e.g. `clayde/issue-{{ number }}-better-branch-naming`). - -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(). - -{"plan": "", "size": "small|large", "branch_name": "clayde/issue-{{ number }}-"} diff --git a/src/clayde/prompts/thorough_plan.j2 b/src/clayde/prompts/thorough_plan.j2 deleted file mode 100644 index fcffca4..0000000 --- a/src/clayde/prompts/thorough_plan.j2 +++ /dev/null @@ -1,47 +0,0 @@ -Research the following GitHub issue and produce an implementation plan. - -ISSUE #{{ number }}: {{ title }} -REPO: {{ owner }}/{{ repo }} -LABELS: {{ labels }} - -ISSUE BODY: -{{ body }} - -PRELIMINARY PLAN (already approved): -{{ preliminary_plan_text }} - -DISCUSSION AFTER PRELIMINARY PLAN: -{{ discussion_text }} - -REPOSITORY ON DISK: {{ repo_path }} - -Before doing anything else, read the following files if they exist in the repository: -- README (any of: README.md, README.rst, README.txt, README) -- agents.md (at the root and any subdirectories relevant to this issue) - -Then explore the repository to understand the codebase relevant to this issue. -Produce a plan that describes WHAT needs to change, not HOW to change it. -The plan will be reviewed by a human, so prioritize clarity and readability. -Use clear structure, short paragraphs, and plain language. - -The plan should include: -- Summary of what needs to be done and why -- Which files and components are affected -- Key design decisions (if any alternatives exist, state which you chose and why) -- Edge cases and risks -- How to verify the implementation is correct - -HARD LIMIT: The plan must be 500 words or fewer. -Do NOT include code snippets, exact code changes, step-by-step implementation -instructions, or line-by-line diffs. The implementing model will read the codebase -itself — your job is to describe the desired outcome clearly, not to dictate the -implementation. - -If anything is ambiguous or you need clarification, include your questions -at the end under a "Questions" section. - -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(). - -{"plan": ""} diff --git a/src/clayde/prompts/update_plan.j2 b/src/clayde/prompts/update_plan.j2 deleted file mode 100644 index 589a882..0000000 --- a/src/clayde/prompts/update_plan.j2 +++ /dev/null @@ -1,43 +0,0 @@ -New comments have been posted on a GitHub issue you are working on. Review the -new comments and update your plan accordingly. - -ISSUE #{{ number }}: {{ title }} -REPO: {{ owner }}/{{ repo }} - -ISSUE BODY: -{{ body }} - -CURRENT PLAN ({{ phase }} plan): -{{ current_plan_text }} - -NEW COMMENTS SINCE LAST UPDATE: -{{ new_comments_text }} - -REPOSITORY ON DISK: {{ repo_path }} - -If you need to re-explore the repository to answer questions or evaluate feedback, -remember to also read the README and agents.md files. - -{% if phase == "preliminary" -%} -IMPORTANT: This is a PRELIMINARY plan — a short overview with questions and a -scope estimate. Do NOT escalate it into a thorough implementation plan. Keep the -same level of detail. Answer questions, incorporate feedback, but stay concise. -{% elif phase == "thorough" -%} -This is a thorough implementation plan. Maintain the same level of detail. -{% endif -%} - -Based on the new comments: -1. Consider whether the current plan needs to be updated -2. Answer any questions directed at you -3. Incorporate any requested changes or clarifications - -Produce a short summary of what changed and your responses to any questions. -Also produce the complete updated plan (copy unchanged sections verbatim — -do not rephrase, reformat, or restructure parts of the plan that are not -affected by the new comments. Only modify what is necessary). - -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": "", "updated_plan": ""} diff --git a/src/clayde/prompts/work.j2 b/src/clayde/prompts/work.j2 new file mode 100644 index 0000000..f6723a8 --- /dev/null +++ b/src/clayde/prompts/work.j2 @@ -0,0 +1,54 @@ +You are Clayde, an autonomous software agent assigned to work on GitHub issues. + +ISSUE #{{ number }}: {{ title }} +REPO: {{ owner }}/{{ repo }} +LABELS: {{ labels }} + +ISSUE BODY: +{{ body }} + +ISSUE COMMENTS (oldest first): +{{ comments_text }} + +{% if pr_url %} +OPEN PULL REQUEST: {{ pr_url }} +PR BRANCH: {{ branch_name }} + +PR REVIEWS: +{{ review_text }} +{% endif %} + +REPOSITORY ON DISK: {{ repo_path }} + +--- + +Read the issue body and comments above, then decide what to do next: + +- If the issue or what is expected of you is unclear, post a clarifying question as a GitHub issue comment using `gh issue comment {{ number }} --repo {{ owner }}/{{ repo }} --body "..."`. +- If you have enough context, implement the solution. Create a branch, implement the changes, run tests, and open a pull request. +- If a PR already exists and there are review comments, address them and push updates to the existing branch. + +When implementing: +1. Before creating a branch, check whether `{{ branch_name }}` already exists locally or on remote — if so, resume from it rather than starting fresh. +2. Read the README (README.md, README.rst, README.txt, or README) and agents.md at the root and in any relevant subdirectories before writing any code. +3. Implement carefully, following existing code style. +4. After implementing, update the README and agents.md as needed to reflect any changes to usage, configuration, architecture, or behavior. +5. Write or update tests if applicable. +6. Run tests if a test command is discoverable (check CLAUDE.md, README, Makefile, package.json, pyproject.toml, justfile): + - If no test runner is discoverable, skip this step. + - If a test suite exists: first note any pre-existing failures, then ensure all tests pass after your changes (pre-existing failures are exempt). Do NOT commit if your changes introduced new test failures. +7. Review your own changes for correctness, edge cases, and style and run "just cleanup" if it exists before committing. +8. Commit with a clear message, e.g.: "Fix #{{ number }}: " +9. Push: git push origin + +When opening a pull request (use `gh pr create --base {{ default_branch }}`): +- Write a concise description summarising what changed and why (include "Closes #{{ number }}" to auto-close the issue). +- If the PR touches more than 3 files, append a **Recommended reading order** section that guides the reviewer through the diff in a logical sequence. + +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": ""} diff --git a/src/clayde/responses.py b/src/clayde/responses.py index 2ba965b..79e20bf 100644 --- a/src/clayde/responses.py +++ b/src/clayde/responses.py @@ -2,31 +2,11 @@ import json import re -from typing import Literal from pydantic import BaseModel, ValidationError -class PreliminaryPlanResponse(BaseModel): - plan: str - size: Literal["small", "large"] - branch_name: str - - -class ThoroughPlanResponse(BaseModel): - plan: str - - -class UpdatePlanResponse(BaseModel): - summary: str - updated_plan: str - - -class ImplementResponse(BaseModel): - summary: str - - -class AddressReviewResponse(BaseModel): +class WorkResponse(BaseModel): summary: str diff --git a/src/clayde/safety.py b/src/clayde/safety.py index 8f94de8..8c7d99b 100644 --- a/src/clayde/safety.py +++ b/src/clayde/safety.py @@ -1,4 +1,4 @@ -"""Safety gates — content filtering and plan approval checks. +"""Safety gates — content filtering. Instead of gatekeeping which issues to work on, we filter *content* so the LLM only sees comments/issue bodies that are created by or approved (👍) @@ -6,7 +6,7 @@ visible content is filtered out the issue is skipped. """ -from github import Github +from datetime import datetime from clayde.config import get_settings @@ -40,17 +40,20 @@ def is_issue_visible(issue) -> bool: return _has_whitelisted_reaction(issue.get_reactions()) -def get_new_visible_comments(comments: list, last_seen_id: int) -> list: - """Return visible comments newer than last_seen_id, excluding Clayde's own. +def get_new_visible_comments(comments: list, last_seen_at: datetime | None) -> list: + """Return visible comments newer than last_seen_at, excluding Clayde's own. - Used by both the orchestrator (to detect when a plan update is needed) - and the plan update task (to collect the new comments for the prompt). + If last_seen_at is None (first time or migrated state), returns all + visible non-Clayde comments. Uses datetime comparison against + comment.created_at. """ github_username = get_settings().github_username visible = filter_comments(comments) + if last_seen_at is None: + return [c for c in visible if c.user.login != github_username] return [ c for c in visible - if c.id > last_seen_id and c.user.login != github_username + if c.created_at > last_seen_at and c.user.login != github_username ] @@ -66,16 +69,6 @@ def has_visible_content(issue, comments: list) -> bool: return False -# --------------------------------------------------------------------------- -# Plan approval -# --------------------------------------------------------------------------- - -def is_plan_approved(g: Github, owner: str, repo: str, number: int, comment_id: int) -> bool: - """Return True if a whitelisted user reacted +1 to the plan comment.""" - comment = g.get_repo(f"{owner}/{repo}").get_issue(number).get_comment(comment_id) - return _has_whitelisted_reaction(comment.get_reactions()) - - # --------------------------------------------------------------------------- # Internal helpers # --------------------------------------------------------------------------- diff --git a/src/clayde/state.py b/src/clayde/state.py index 7cc312e..81a06aa 100644 --- a/src/clayde/state.py +++ b/src/clayde/state.py @@ -2,9 +2,6 @@ import json import logging -from enum import StrEnum - -from opentelemetry import trace from clayde.config import DATA_DIR @@ -13,19 +10,6 @@ _STATE_FILE = DATA_DIR / "state.json" -class IssueStatus(StrEnum): - PRELIMINARY_PLANNING = "preliminary_planning" - AWAITING_PRELIMINARY_APPROVAL = "awaiting_preliminary_approval" - PLANNING = "planning" - AWAITING_PLAN_APPROVAL = "awaiting_plan_approval" - IMPLEMENTING = "implementing" - PR_OPEN = "pr_open" - ADDRESSING_REVIEW = "addressing_review" - DONE = "done" - FAILED = "failed" - INTERRUPTED = "interrupted" - - def load_state() -> dict: if _STATE_FILE.exists(): return json.loads(_STATE_FILE.read_text()) @@ -43,33 +27,5 @@ def get_issue_state(issue_url: str) -> dict: def update_issue_state(issue_url: str, updates: dict) -> None: state = load_state() entry = state["issues"].setdefault(issue_url, {}) - old_status = entry.get("status") entry.update(updates) - new_status = entry.get("status") - save_state(state) - - if old_status != new_status: - span = trace.get_current_span() - if span.is_recording(): - span.add_event("state_transition", attributes={ - "issue.url": issue_url, - "old_status": old_status or "(none)", - "new_status": new_status or "(none)", - }) - - -def accumulate_cost(issue_url: str, cost_eur: float) -> None: - """Add cost to the running total for this issue.""" - state = load_state() - entry = state["issues"].setdefault(issue_url, {}) - entry["accumulated_cost_eur"] = entry.get("accumulated_cost_eur", 0.0) + cost_eur - save_state(state) - - -def pop_accumulated_cost(issue_url: str) -> float: - """Return and reset the accumulated cost for this issue.""" - state = load_state() - entry = state["issues"].get(issue_url, {}) - cost = entry.pop("accumulated_cost_eur", 0.0) save_state(state) - return cost diff --git a/src/clayde/tasks/implement.py b/src/clayde/tasks/implement.py deleted file mode 100644 index 4023b83..0000000 --- a/src/clayde/tasks/implement.py +++ /dev/null @@ -1,342 +0,0 @@ -"""Implement task — implement the approved plan, open PR, assign reviewer. - -After implementation, assigns the issue author as PR reviewer and sets -status to ``pr_open`` for review monitoring. -""" - -import logging -import subprocess -from pathlib import Path - -from clayde.claude import InvocationTimeoutError, UsageLimitError, format_cost_line, invoke_claude -from clayde.config import DATA_DIR, get_github_client, get_settings -from clayde.git import ensure_repo -from clayde.prompts import collect_comments_after, render_template -from clayde.github import ( - add_pr_reviewer, - create_pull_request, - fetch_comment, - fetch_issue, - fetch_issue_comments, - find_open_pr, - get_default_branch, - get_issue_author, - get_pr_title, - issue_ref, - parse_issue_url, - parse_pr_url, - post_comment, -) -from clayde.responses import ImplementResponse, parse_response -from clayde.safety import filter_comments -from clayde.state import IssueStatus, accumulate_cost, get_issue_state, pop_accumulated_cost, update_issue_state -from clayde.telemetry import get_tracer - -log = logging.getLogger("clayde.tasks.implement") - - -def _delete_conversation_file(owner: str, repo: str, number: int) -> None: - """Remove the conversation file for an issue so stale sessions don't persist.""" - path = DATA_DIR / "conversations" / f"{owner}__{repo}__issue-{number}.json" - if path.exists(): - path.unlink() - log.info("Deleted conversation file %s", path) - - -def run(issue_url: str) -> None: - tracer = get_tracer() - with tracer.start_as_current_span("clayde.task.implement") as span: - g = get_github_client() - owner, repo, number = parse_issue_url(issue_url) - issue_state = get_issue_state(issue_url) - span.set_attribute("issue.number", number) - span.set_attribute("issue.owner", owner) - span.set_attribute("issue.repo", repo) - - resumed = issue_state.get("status") == IssueStatus.INTERRUPTED - span.set_attribute("implement.resumed_from_interrupted", resumed) - - if resumed and _try_resume_from_existing_pr(g, owner, repo, number, issue_url, issue_state, span): - return - - if not resumed: - _delete_conversation_file(owner, repo, number) - - update_issue_state(issue_url, {"status": IssueStatus.IMPLEMENTING}) - - issue, default_branch, repo_path, branch_name, prompt, conversation_path = ( - _prepare_implementation_context(g, owner, repo, number, issue_url, issue_state, resumed) - ) - - log.info("[%s: %s] Invoking Claude for implementation", issue_ref(owner, repo, number), issue.title) - try: - result = invoke_claude( - prompt, - repo_path, - branch_name=branch_name, - conversation_path=conversation_path, - ) - except UsageLimitError as e: - log.warning("[%s: %s] Usage limit hit during implementation — will retry next cycle", issue_ref(owner, repo, number), issue.title) - accumulate_cost(issue_url, e.cost_eur) - span.set_attribute("implement.status", "limit") - update_issue_state(issue_url, { - "status": IssueStatus.INTERRUPTED, - "interrupted_phase": IssueStatus.IMPLEMENTING, - }) - return - except InvocationTimeoutError as e: - log.warning("[%s: %s] Timed out during implementation — will resume next cycle", issue_ref(owner, repo, number), issue.title) - accumulate_cost(issue_url, e.cost_eur) - span.set_attribute("implement.status", "timeout") - update_issue_state(issue_url, { - "status": IssueStatus.INTERRUPTED, - "interrupted_phase": IssueStatus.IMPLEMENTING, - }) - return - - total_cost = pop_accumulated_cost(issue_url) + result.cost_eur - - # Parse JSON response to extract summary - summary = None - try: - parsed = parse_response(result.output, ImplementResponse) - summary = parsed.summary - except ValueError as e: - log.warning("[%s: %s] Failed to parse implement response JSON: %s", issue_ref(owner, repo, number), issue.title, e) - - pr_url = _find_or_create_pr(g, owner, repo, number, branch_name, default_branch, total_cost, issue, - repo_path=repo_path, summary=summary) - - if pr_url: - _assign_reviewer_and_finish(g, owner, repo, number, issue_url, pr_url, span, cost_eur=total_cost) - else: - log.error("[%s: %s] Implementation produced no PR", issue_ref(owner, repo, number), issue.title) - log.error("Claude output (last 2000 chars): %s", (result.output or "")[-2000:]) - _handle_no_pr(g, owner, repo, number, issue_url, issue_state, span) - - -def _try_resume_from_existing_pr(g, owner, repo, number, issue_url, issue_state, span) -> bool: - """If a PR already exists for the WIP branch, finish without re-running Claude. - - Returns True if we found an existing PR and finished; False otherwise. - """ - branch_name = issue_state.get("branch_name", f"clayde/issue-{number}") - existing_pr = find_open_pr(g, owner, repo, branch_name) - if not existing_pr: - return False - - log.info("Resuming interrupted #%d — found existing PR %s", number, existing_pr) - accumulated = pop_accumulated_cost(issue_url) - _assign_reviewer_and_finish( - g, owner, repo, number, issue_url, existing_pr, span, - cost_eur=accumulated if accumulated > 0 else None, - ) - return True - - -def _prepare_implementation_context(g, owner, repo, number, issue_url, issue_state, resumed): - """Fetch all resources needed to run Claude and return them as a tuple.""" - plan_comment_id = issue_state.get("plan_comment_id") or issue_state["preliminary_comment_id"] - issue = fetch_issue(g, owner, repo, number) - default_branch = get_default_branch(g, owner, repo) - repo_path = ensure_repo(owner, repo, default_branch) - - plan_comment = fetch_comment(g, owner, repo, number, plan_comment_id) - plan_text = plan_comment.body - - branch_name = issue_state.get("branch_name") or f"clayde/issue-{number}" - update_issue_state(issue_url, {"branch_name": branch_name}) - - if resumed: - _checkout_wip_branch(repo_path, branch_name) - - all_comments = fetch_issue_comments(g, owner, repo, number) - visible_comments = filter_comments(all_comments) - discussion_text = collect_comments_after(visible_comments, plan_comment_id) - - prompt = _build_prompt(issue, plan_text, discussion_text, owner, repo, number, repo_path, branch_name) - - conversation_path = DATA_DIR / "conversations" / f"{owner}__{repo}__issue-{number}.json" - return issue, default_branch, repo_path, branch_name, prompt, conversation_path - - -def _ensure_branch_pushed(repo_path: Path, branch_name: str) -> bool: - """Check if branch exists on remote; if not, try to push it. - - Returns True if the branch is on the remote after this call. - """ - result = subprocess.run( - ["git", "ls-remote", "--heads", "origin", branch_name], - cwd=str(repo_path), capture_output=True, text=True, - ) - if result.stdout.strip(): - return True - - # Branch not on remote — check if it exists locally and push it - result = subprocess.run( - ["git", "branch", "--list", branch_name], - cwd=str(repo_path), capture_output=True, text=True, - ) - if not result.stdout.strip(): - log.error("Branch %s does not exist locally or on remote", branch_name) - return False - - log.info("Branch %s exists locally but not on remote — pushing", branch_name) - result = subprocess.run( - ["git", "push", "origin", branch_name], - cwd=str(repo_path), capture_output=True, text=True, - ) - if result.returncode != 0: - log.error("Failed to push branch %s: %s", branch_name, result.stderr) - return False - - return True - - -def _find_or_create_pr(g, owner, repo, number, branch_name, default_branch, total_cost, issue, - repo_path: Path | None = None, summary: str | None = None) -> str | None: - """Return the PR URL for branch_name, creating it if necessary.""" - pr_url = find_open_pr(g, owner, repo, branch_name) - if pr_url: - return pr_url - - if repo_path and not _ensure_branch_pushed(repo_path, branch_name): - log.error("Cannot create PR for #%d: branch %s not available on remote", number, branch_name) - return None - - try: - if summary: - pr_body = f"Closes #{number}\n\n{summary}{format_cost_line(total_cost)}" - else: - pr_body = f"Closes #{number}{format_cost_line(total_cost)}" - pr_url = create_pull_request( - g, owner, repo, - title=f"Fix #{number}: {issue.title}", - body=pr_body, - head=branch_name, - base=default_branch, - ) - log.info("Created PR: %s", pr_url) - return pr_url - except Exception as e: - log.error("Failed to create PR for #%d: %s", number, e) - return None - - -def _handle_no_pr(g, owner, repo, number, issue_url, issue_state, span) -> None: - """Handle the case where implementation produced no PR — retry or give up.""" - max_retries = get_settings().implement_max_retries - retry_count = issue_state.get("retry_count", 0) + 1 - if retry_count >= max_retries: - log.error("Issue #%d failed after %d retries — giving up", number, retry_count) - _delete_conversation_file(owner, repo, number) - post_comment(g, owner, repo, number, - "Implementation failed to produce a PR after multiple retries. " - "Marking as failed — manual intervention needed.") - span.set_attribute("implement.status", "failed") - update_issue_state(issue_url, {"status": IssueStatus.FAILED, "retry_count": retry_count}) - else: - post_comment(g, owner, repo, number, - f"Implementation ran but no PR was created " - f"(attempt {retry_count}/{max_retries}). " - "I'll retry on the next cycle.") - span.set_attribute("implement.status", "no_pr") - span.set_attribute("implement.retry_count", retry_count) - update_issue_state(issue_url, { - "status": IssueStatus.INTERRUPTED, - "interrupted_phase": IssueStatus.IMPLEMENTING, - "retry_count": retry_count, - }) - - -def _assign_reviewer_and_finish(g, owner, repo, number, issue_url, pr_url, span, - cost_eur=None): - """Post result, assign reviewer, set status to pr_open.""" - _post_result(g, owner, repo, number, pr_url, cost_eur=cost_eur) - - _, _, pr_number = parse_pr_url(pr_url) - - pr_title = None - try: - pr_title = get_pr_title(g, owner, repo, pr_number) - except Exception as e: - log.warning("Failed to fetch PR title for PR %s: %s", pr_url, e) - - try: - issue_author = get_issue_author(g, owner, repo, number) - settings = get_settings() - if issue_author.lower() == settings.github_username.lower(): - log.info("Issue author is %s (self) — skipping reviewer assignment", issue_author) - else: - add_pr_reviewer(g, owner, repo, pr_number, issue_author) - except Exception as e: - log.warning("Failed to assign reviewer for PR %s: %s", pr_url, e) - - state_update = { - "status": IssueStatus.PR_OPEN, - "pr_url": pr_url, - "last_seen_review_id": 0, - } - if pr_title: - state_update["pr_title"] = pr_title - - update_issue_state(issue_url, state_update) - span.set_attribute("implement.status", "pr_open") - span.set_attribute("implement.pr_url", pr_url) - ref = issue_ref(owner, repo, number) - label = f"{ref}: {pr_title}" if pr_title else ref - log.info("[%s] PR open — monitoring for reviews: %s", label, pr_url) - - -def _checkout_wip_branch(repo_path, branch_name: str) -> None: - """Checkout an existing WIP branch if it exists (locally or on remote).""" - cwd = str(repo_path) - - result = subprocess.run( - ["git", "branch", "--list", branch_name], - cwd=cwd, capture_output=True, text=True, - ) - if result.stdout.strip(): - subprocess.run(["git", "checkout", branch_name], cwd=cwd, capture_output=True) - subprocess.run(["git", "pull", "origin", branch_name], cwd=cwd, capture_output=True) - log.info("Resumed WIP branch %s (local)", branch_name) - return - - result = subprocess.run( - ["git", "ls-remote", "--heads", "origin", branch_name], - cwd=cwd, capture_output=True, text=True, - ) - if result.stdout.strip(): - subprocess.run( - ["git", "checkout", "-b", branch_name, f"origin/{branch_name}"], - cwd=cwd, capture_output=True, - ) - log.info("Resumed WIP branch %s (remote)", branch_name) - return - - log.info("No existing WIP branch %s found — starting fresh", branch_name) - - -def _build_prompt(issue, plan_text: str, discussion_text: str, owner: str, repo: str, - number: int, repo_path: str, branch_name: str) -> str: - return render_template( - "implement.j2", - number=number, - title=issue.title, - owner=owner, - repo=repo, - body=issue.body or "(empty)", - plan_text=plan_text, - discussion_text=discussion_text, - repo_path=repo_path, - branch_name=branch_name, - ) - - -def _post_result(g, owner: str, repo: str, number: int, pr_url: str, - cost_eur: float | None = None) -> None: - body = f"Implementation complete — PR opened: {pr_url}" - if cost_eur is not None: - body += format_cost_line(cost_eur) - post_comment(g, owner, repo, number, body) diff --git a/src/clayde/tasks/plan.py b/src/clayde/tasks/plan.py deleted file mode 100644 index 846636e..0000000 --- a/src/clayde/tasks/plan.py +++ /dev/null @@ -1,425 +0,0 @@ -"""Plan task — two-phase planning with preliminary and thorough plans. - -Phase 1 (preliminary): Post a short overview with questions. -Phase 2 (thorough): After preliminary approval, post the full detailed plan. -Updates: When new visible comments arrive, update the current plan and post a - summary of changes. -""" - -import logging - -from github import Github -from github.Issue import Issue - -from clayde.claude import InvocationTimeoutError, UsageLimitError, format_cost_line, invoke_claude -from clayde.config import get_github_client -from clayde.git import ensure_repo -from clayde.github import ( - edit_comment, - fetch_comment, - fetch_issue, - fetch_issue_comments, - get_default_branch, - issue_ref, - parse_issue_url, - post_comment, -) -from clayde.prompts import collect_comments_after, render_template -from clayde.responses import ( - PreliminaryPlanResponse, - ThoroughPlanResponse, - UpdatePlanResponse, - parse_response, -) -from clayde.safety import filter_comments, get_new_visible_comments, is_issue_visible -from clayde.state import IssueStatus, accumulate_cost, get_issue_state, pop_accumulated_cost, update_issue_state -from clayde.telemetry import get_tracer - -log = logging.getLogger("clayde.tasks.plan") - - -# --------------------------------------------------------------------------- -# Phase 1: Preliminary plan -# --------------------------------------------------------------------------- - -def run_preliminary(issue_url: str) -> None: - """Research the issue and post a short preliminary plan with questions.""" - tracer = get_tracer() - with tracer.start_as_current_span("clayde.task.preliminary_plan") as span: - g = get_github_client() - owner, repo, number = parse_issue_url(issue_url) - span.set_attribute("issue.number", number) - span.set_attribute("issue.owner", owner) - span.set_attribute("issue.repo", repo) - issue = fetch_issue(g, owner, repo, number) - update_issue_state(issue_url, { - "status": IssueStatus.PRELIMINARY_PLANNING, - "owner": owner, "repo": repo, "number": number, - "issue_title": issue.title, - }) - default_branch = get_default_branch(g, owner, repo) - repo_path = ensure_repo(owner, repo, default_branch) - - prompt = _build_preliminary_prompt(g, issue, owner, repo, number, repo_path) - - log.info("[%s: %s] Invoking Claude for preliminary plan", issue_ref(owner, repo, number), issue.title) - try: - result = invoke_claude(prompt, repo_path) - except (UsageLimitError, InvocationTimeoutError) as e: - label = "Timed out" if isinstance(e, InvocationTimeoutError) else "Usage limit hit" - log.warning("%s during preliminary planning #%d", label, number) - accumulate_cost(issue_url, e.cost_eur) - span.set_attribute("plan.status", "timeout" if isinstance(e, InvocationTimeoutError) else "limit") - update_issue_state(issue_url, { - "status": IssueStatus.INTERRUPTED, - "interrupted_phase": IssueStatus.PRELIMINARY_PLANNING, - }) - return - - raw_output = result.output - total_cost = pop_accumulated_cost(issue_url) + result.cost_eur - span.set_attribute("plan.output_length", len(raw_output)) - - if not raw_output.strip(): - log.error("Claude returned empty preliminary plan for #%d: %s", number, issue.title) - span.set_attribute("plan.status", "empty") - update_issue_state(issue_url, {"status": IssueStatus.FAILED}) - return - - try: - parsed = parse_response(raw_output, PreliminaryPlanResponse) - except ValueError as e: - log.error("Failed to parse preliminary plan response for #%d: %s: %s", number, issue.title, e) - span.set_attribute("plan.status", "parse_error") - update_issue_state(issue_url, {"status": IssueStatus.FAILED}) - return - - plan_text = parsed.plan - size = parsed.size - branch_name = parsed.branch_name - - if len(plan_text.strip()) < 100: - log.warning("Claude returned very short preliminary plan for #%d: %s (%d chars)", - number, issue.title, len(plan_text.strip())) - span.set_attribute("plan.status", "short") - update_issue_state(issue_url, { - "status": IssueStatus.INTERRUPTED, - "interrupted_phase": IssueStatus.PRELIMINARY_PLANNING, - }) - return - - comment_id = _post_preliminary_comment(g, owner, repo, number, plan_text, total_cost, size=size) - - # Track the last seen comment so we can detect new ones later - all_comments = fetch_issue_comments(g, owner, repo, number) - last_comment_id = all_comments[-1].id if all_comments else 0 - - update_issue_state(issue_url, { - "status": IssueStatus.AWAITING_PRELIMINARY_APPROVAL, - "preliminary_comment_id": comment_id, - "last_seen_comment_id": last_comment_id, - "size": size, - "branch_name": branch_name, - }) - span.set_attribute("plan.status", "posted") - log.info("[%s: %s] Preliminary plan posted (comment %s)", issue_ref(owner, repo, number), issue.title, comment_id) - - -# --------------------------------------------------------------------------- -# Phase 2: Thorough plan -# --------------------------------------------------------------------------- - -def run_thorough(issue_url: str) -> None: - """Post a thorough implementation plan (after preliminary approval).""" - tracer = get_tracer() - with tracer.start_as_current_span("clayde.task.thorough_plan") as span: - g = get_github_client() - owner, repo, number = parse_issue_url(issue_url) - span.set_attribute("issue.number", number) - span.set_attribute("issue.owner", owner) - span.set_attribute("issue.repo", repo) - update_issue_state(issue_url, {"status": IssueStatus.PLANNING}) - - issue = fetch_issue(g, owner, repo, number) - default_branch = get_default_branch(g, owner, repo) - repo_path = ensure_repo(owner, repo, default_branch) - - issue_state = get_issue_state(issue_url) - preliminary_comment_id = issue_state.get("preliminary_comment_id") - preliminary_comment = fetch_comment(g, owner, repo, number, preliminary_comment_id) - preliminary_text = preliminary_comment.body - - # Collect discussion after the preliminary plan - all_comments = fetch_issue_comments(g, owner, repo, number) - visible_comments = filter_comments(all_comments) - discussion_text = collect_comments_after(visible_comments, preliminary_comment_id) - - prompt = _build_thorough_prompt( - g, issue, owner, repo, number, repo_path, - preliminary_text, discussion_text, - ) - - log.info("[%s: %s] Invoking Claude for thorough plan", issue_ref(owner, repo, number), issue.title) - try: - result = invoke_claude(prompt, repo_path) - except (UsageLimitError, InvocationTimeoutError) as e: - label = "Timed out" if isinstance(e, InvocationTimeoutError) else "Usage limit hit" - log.warning("%s during thorough planning #%d", label, number) - accumulate_cost(issue_url, e.cost_eur) - span.set_attribute("plan.status", "timeout" if isinstance(e, InvocationTimeoutError) else "limit") - update_issue_state(issue_url, { - "status": IssueStatus.INTERRUPTED, - "interrupted_phase": IssueStatus.PLANNING, - }) - return - - raw_output = result.output - total_cost = pop_accumulated_cost(issue_url) + result.cost_eur - span.set_attribute("plan.output_length", len(raw_output)) - - if not raw_output.strip(): - log.error("Claude returned empty thorough plan for #%d: %s", number, issue.title) - span.set_attribute("plan.status", "empty") - update_issue_state(issue_url, {"status": IssueStatus.FAILED}) - return - - try: - parsed = parse_response(raw_output, ThoroughPlanResponse) - except ValueError as e: - log.error("Failed to parse thorough plan response for #%d: %s: %s", number, issue.title, e) - span.set_attribute("plan.status", "parse_error") - update_issue_state(issue_url, {"status": IssueStatus.FAILED}) - return - - plan_text = parsed.plan - - if len(plan_text.strip()) < 200: - log.warning("Claude returned very short thorough plan for #%d: %s (%d chars)", - number, issue.title, len(plan_text.strip())) - span.set_attribute("plan.status", "short") - update_issue_state(issue_url, { - "status": IssueStatus.INTERRUPTED, - "interrupted_phase": IssueStatus.PLANNING, - }) - return - - comment_id = _post_thorough_plan_comment(g, owner, repo, number, plan_text, total_cost) - - # Update last seen comment - all_comments = fetch_issue_comments(g, owner, repo, number) - last_comment_id = all_comments[-1].id if all_comments else 0 - - update_issue_state(issue_url, { - "status": IssueStatus.AWAITING_PLAN_APPROVAL, - "plan_comment_id": comment_id, - "last_seen_comment_id": last_comment_id, - }) - span.set_attribute("plan.status", "posted") - log.info("[%s: %s] Thorough plan posted (comment %s)", issue_ref(owner, repo, number), issue.title, comment_id) - - -# --------------------------------------------------------------------------- -# Plan update (new comments detected) -# --------------------------------------------------------------------------- - -def run_update(issue_url: str, phase: str) -> None: - """Process new visible comments and update the current plan. - - Args: - issue_url: The issue URL. - phase: Either "preliminary" or "thorough" — which plan to update. - """ - tracer = get_tracer() - with tracer.start_as_current_span("clayde.task.update_plan") as span: - g = get_github_client() - owner, repo, number = parse_issue_url(issue_url) - span.set_attribute("issue.number", number) - span.set_attribute("plan.update_phase", phase) - - issue_state = get_issue_state(issue_url) - - if phase == "preliminary": - plan_comment_id = issue_state.get("preliminary_comment_id") - return_status = IssueStatus.AWAITING_PRELIMINARY_APPROVAL - elif phase == "thorough": - plan_comment_id = issue_state.get("plan_comment_id") - return_status = IssueStatus.AWAITING_PLAN_APPROVAL - else: - raise ValueError(f"Unknown plan update phase: {phase!r}") - - last_seen = issue_state.get("last_seen_comment_id", 0) - - issue = fetch_issue(g, owner, repo, number) - default_branch = get_default_branch(g, owner, repo) - repo_path = ensure_repo(owner, repo, default_branch) - - plan_comment = fetch_comment(g, owner, repo, number, plan_comment_id) - current_plan_text = plan_comment.body - - all_comments = fetch_issue_comments(g, owner, repo, number) - new_comments = get_new_visible_comments(all_comments, last_seen) - - if not new_comments: - log.info("[%s: %s] No new visible comments — skipping update", issue_ref(owner, repo, number), issue.title) - return - - new_comments_text = "\n---\n".join( - f"@{c.user.login}:\n{c.body}" for c in new_comments - ) - - body_text = issue.body or "(empty)" - if not is_issue_visible(issue): - body_text = "(filtered)" - - prompt = _build_update_prompt( - number, issue.title, owner, repo, - body_text, current_plan_text, new_comments_text, repo_path, - phase=phase, - ) - - log.info("[%s: %s] Invoking Claude for plan update (%s phase)", issue_ref(owner, repo, number), issue.title, phase) - try: - result = invoke_claude(prompt, repo_path) - except (UsageLimitError, InvocationTimeoutError) as e: - label = "Timed out" if isinstance(e, InvocationTimeoutError) else "Usage limit hit" - log.warning("%s during plan update #%d", label, number) - accumulate_cost(issue_url, e.cost_eur) - span.set_attribute("plan.update_status", "timeout" if isinstance(e, InvocationTimeoutError) else "limit") - return - - total_cost = pop_accumulated_cost(issue_url) + result.cost_eur - - # Parse JSON output into summary + updated plan - try: - parsed = parse_response(result.output, UpdatePlanResponse) - summary = parsed.summary - updated_plan = parsed.updated_plan - except ValueError as e: - log.error("[%s: %s] Failed to parse update plan response: %s", issue_ref(owner, repo, number), issue.title, e) - span.set_attribute("plan.update_status", "parse_error") - return - - if updated_plan: - # Edit the existing plan comment - edit_comment(g, owner, repo, number, plan_comment_id, updated_plan) - log.info("[%s: %s] Updated %s plan comment %d", issue_ref(owner, repo, number), issue.title, phase, plan_comment_id) - - if summary: - # Post a new comment with the change summary - post_comment(g, owner, repo, number, - f"**Plan updated.** {summary}{format_cost_line(total_cost)}") - - # Update last seen comment - all_comments = fetch_issue_comments(g, owner, repo, number) - last_comment_id = all_comments[-1].id if all_comments else 0 - - update_issue_state(issue_url, { - "status": return_status, - "last_seen_comment_id": last_comment_id, - }) - span.set_attribute("plan.update_status", "updated") - log.info("[%s: %s] Plan update complete", issue_ref(owner, repo, number), issue.title) - - - -# --------------------------------------------------------------------------- -# Prompt builders -# --------------------------------------------------------------------------- - -def _build_preliminary_prompt(g: Github, issue: Issue, owner: str, repo: str, number: int, repo_path: str) -> str: - labels = ", ".join(l.name for l in issue.labels) or "none" - comments = fetch_issue_comments(g, owner, repo, number) - visible = filter_comments(comments) - comments_text = "\n".join( - f"@{c.user.login}:\n{c.body}\n---" for c in visible - ) or "(none)" - - body_text = issue.body or "(empty)" - if not is_issue_visible(issue): - body_text = "(filtered)" - - return render_template( - "preliminary_plan.j2", - number=number, - title=issue.title, - owner=owner, - repo=repo, - labels=labels, - body=body_text, - comments_text=comments_text, - repo_path=repo_path, - ) - - -def _build_thorough_prompt(g: Github, issue: Issue, owner: str, repo: str, number: int, - repo_path: str, preliminary_text: str, discussion_text: str) -> str: - labels = ", ".join(l.name for l in issue.labels) or "none" - - body_text = issue.body or "(empty)" - if not is_issue_visible(issue): - body_text = "(filtered)" - - return render_template( - "thorough_plan.j2", - number=number, - title=issue.title, - owner=owner, - repo=repo, - labels=labels, - body=body_text, - preliminary_plan_text=preliminary_text, - discussion_text=discussion_text, - repo_path=repo_path, - ) - - -def _build_update_prompt(number: int, title: str, owner: str, repo: str, body: str, - current_plan_text: str, new_comments_text: str, repo_path: str, - phase: str) -> str: - return render_template( - "update_plan.j2", - number=number, - title=title, - owner=owner, - repo=repo, - body=body, - current_plan_text=current_plan_text, - new_comments_text=new_comments_text, - repo_path=repo_path, - phase=phase, - ) - - -# --------------------------------------------------------------------------- -# Comment formatting -# --------------------------------------------------------------------------- - -def _post_preliminary_comment(g, owner: str, repo: str, number: int, plan_text: str, - cost_eur: float = 0.0, *, size: str = "large") -> int: - if size == "small": - next_step = "proceed directly to implementation" - else: - next_step = "proceed to a thorough implementation plan" - comment_body = ( - f"## Preliminary Plan\n\n" - f"{plan_text}\n\n" - f"---\n" - f"*This looks like a **{size}** issue. " - f"React with \U0001f44d to approve this preliminary plan and {next_step}.*" - f"{format_cost_line(cost_eur)}" - ) - return post_comment(g, owner, repo, number, comment_body) - - -def _post_thorough_plan_comment(g, owner: str, repo: str, number: int, plan_text: str, - cost_eur: float = 0.0) -> int: - comment_body = ( - f"## Implementation Plan\n\n" - f"{plan_text}\n\n" - f"---\n" - f"*React with \U0001f44d to approve this plan and start implementation.*" - f"{format_cost_line(cost_eur)}" - ) - return post_comment(g, owner, repo, number, comment_body) - - diff --git a/src/clayde/tasks/review.py b/src/clayde/tasks/review.py deleted file mode 100644 index 0d4a102..0000000 --- a/src/clayde/tasks/review.py +++ /dev/null @@ -1,185 +0,0 @@ -"""Review task — address PR review comments and push updates.""" - -import logging - -from clayde.claude import InvocationTimeoutError, UsageLimitError, format_cost_line, invoke_claude -from clayde.config import DATA_DIR, get_github_client, get_settings -from clayde.git import ensure_repo -from clayde.prompts import render_template -from clayde.github import ( - fetch_issue, - get_default_branch, - get_pr_review_comments, - get_pr_reviews, - issue_ref, - parse_issue_url, - parse_pr_url, - post_comment, -) -from clayde.responses import AddressReviewResponse, parse_response -from clayde.state import IssueStatus, accumulate_cost, get_issue_state, pop_accumulated_cost, update_issue_state -from clayde.telemetry import get_tracer - -log = logging.getLogger("clayde.tasks.review") - - -def run(issue_url: str) -> None: - """Check for new PR reviews, address comments, push updates.""" - tracer = get_tracer() - with tracer.start_as_current_span("clayde.task.review") as span: - g = get_github_client() - owner, repo, number = parse_issue_url(issue_url) - issue_state = get_issue_state(issue_url) - pr_url = issue_state.get("pr_url") - span.set_attribute("issue.number", number) - span.set_attribute("issue.owner", owner) - span.set_attribute("issue.repo", repo) - - issue_title = issue_state.get("pr_title") or issue_state.get("issue_title") or "(title unknown)" - issue_label = f"{issue_ref(owner, repo, number)}: {issue_title}" - - if not pr_url: - log.warning("[%s] No PR URL in state — skipping review", issue_label) - return - - _, _, pr_number = parse_pr_url(pr_url) - last_seen_review_id = issue_state.get("last_seen_review_id", 0) - github_username = get_settings().github_username - - # Get all reviews and filter to new ones - reviews = get_pr_reviews(g, owner, repo, pr_number) - new_reviews = [ - r for r in reviews - if r.id > last_seen_review_id - and r.user.login != github_username - ] - - if not new_reviews: - log.info("[%s] No new reviews on PR #%d", issue_label, pr_number) - return - - # Check if any new review has actual content (comments or body) - review_comments = get_pr_review_comments(g, owner, repo, pr_number) - new_review_ids = {r.id for r in new_reviews} - relevant_comments = [ - rc for rc in review_comments - if rc.pull_request_review_id in new_review_ids - ] - - # Also include reviews with body text - review_bodies = [ - r for r in new_reviews - if r.body and r.body.strip() - ] - - if not relevant_comments and not review_bodies: - # Reviews with no comments (e.g. just "approved") — update seen ID - max_review_id = max(r.id for r in new_reviews) - update_issue_state(issue_url, {"last_seen_review_id": max_review_id}) - - # Check if any review is an approval - approved = any(r.state == "APPROVED" for r in new_reviews) - if approved: - log.info("[%s] PR #%d approved — marking as done", issue_label, pr_number) - update_issue_state(issue_url, {"status": IssueStatus.DONE}) - span.set_attribute("review.status", "approved") - return - - # Build review text for Claude - review_text = _format_reviews(new_reviews, relevant_comments) - - log.info("[%s] Addressing %d new review(s) on PR #%d", - issue_label, len(new_reviews), pr_number) - - update_issue_state(issue_url, {"status": IssueStatus.ADDRESSING_REVIEW}) - - issue = fetch_issue(g, owner, repo, number) - default_branch = get_default_branch(g, owner, repo) - repo_path = ensure_repo(owner, repo, default_branch) - branch_name = issue_state.get("branch_name", f"clayde/issue-{number}") - - prompt = _build_prompt( - issue, owner, repo, number, repo_path, branch_name, review_text, - ) - - conversation_path = DATA_DIR / "conversations" / f"{owner}__{repo}__issue-{number}-review.json" - - log.info("[%s] Invoking Claude to address review", issue_label) - try: - result = invoke_claude( - prompt, - repo_path, - branch_name=branch_name, - conversation_path=conversation_path, - ) - except (UsageLimitError, InvocationTimeoutError) as e: - label_msg = "Timed out" if isinstance(e, InvocationTimeoutError) else "Usage limit hit" - log.warning("[%s] %s during review handling", issue_label, label_msg) - accumulate_cost(issue_url, e.cost_eur) - span.set_attribute("review.status", "timeout" if isinstance(e, InvocationTimeoutError) else "limit") - update_issue_state(issue_url, { - "status": IssueStatus.INTERRUPTED, - "interrupted_phase": IssueStatus.ADDRESSING_REVIEW, - }) - return - - total_cost = pop_accumulated_cost(issue_url) + result.cost_eur - - # Parse JSON response to extract summary - summary = None - try: - parsed = parse_response(result.output, AddressReviewResponse) - summary = parsed.summary - except ValueError as e: - log.warning("[%s] Failed to parse address_review response JSON: %s", issue_label, e) - # Fall back to raw output - summary = result.output.strip() if result.output else None - - # Post summary comment on the issue - if summary: - post_comment(g, owner, repo, number, - f"**Review addressed.** {summary}{format_cost_line(total_cost)}") - - # Update last seen review ID and return to pr_open - max_review_id = max(r.id for r in new_reviews) - update_issue_state(issue_url, { - "status": IssueStatus.PR_OPEN, - "last_seen_review_id": max_review_id, - }) - span.set_attribute("review.status", "addressed") - log.info("[%s] Review comments addressed", issue_label) - - -def _format_reviews(reviews: list, review_comments: list) -> str: - """Format reviews and review comments into text for the prompt.""" - parts = [] - - for review in reviews: - header = f"Review by @{review.user.login} (state: {review.state}):" - inline_comments = [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_comments: - parts.append(f"{header}\n{review.body}" if has_body else header) - - for rc in inline_comments: - 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 "(no review content)" - - -def _build_prompt(issue, owner, repo, number, repo_path, branch_name, review_text) -> str: - return render_template( - "address_review.j2", - number=number, - title=issue.title, - owner=owner, - repo=repo, - body=issue.body or "(empty)", - branch_name=branch_name, - review_text=review_text, - repo_path=repo_path, - ) diff --git a/src/clayde/tasks/work.py b/src/clayde/tasks/work.py new file mode 100644 index 0000000..87eda02 --- /dev/null +++ b/src/clayde/tasks/work.py @@ -0,0 +1,166 @@ +"""Work task — unified event-driven work on a GitHub issue. + +Claude decides what to do next: ask questions, plan, implement, or address +reviews. PR creation is handled by Claude via `gh pr create`; Python detects +the resulting PR via find_open_pr() and persists it in state. +""" + +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 ( + add_pr_reviewer, + fetch_issue, + fetch_issue_comments, + find_open_pr, + get_default_branch, + get_issue_author, + get_pr_review_comments, + get_pr_reviews, + issue_ref, + parse_issue_url, + parse_pr_url, + post_comment, +) +from clayde.prompts import render_template +from clayde.responses import WorkResponse, parse_response +from clayde.safety import filter_comments, is_issue_visible +from clayde.state import get_issue_state, update_issue_state +from clayde.telemetry import get_tracer + +log = logging.getLogger("clayde.tasks.work") + + +def run(issue_url: str) -> None: + """Gather full issue context and invoke Claude to take the next action. + + 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.work") as span: + g = get_github_client() + owner, repo, number = parse_issue_url(issue_url) + issue_state = get_issue_state(issue_url) + span.set_attribute("issue.number", number) + span.set_attribute("issue.owner", owner) + span.set_attribute("issue.repo", repo) + + issue = fetch_issue(g, owner, repo, number) + default_branch = get_default_branch(g, owner, repo) + repo_path = ensure_repo(owner, repo, default_branch) + + branch_name = issue_state.get("branch_name") or f"clayde/issue-{number}" + pr_url = issue_state.get("pr_url") + + # Build visible comment text + all_comments = fetch_issue_comments(g, owner, repo, number) + visible_comments = filter_comments(all_comments) + comments_text = "\n---\n".join( + f"@{c.user.login}:\n{c.body}" for c in visible_comments + ) or "(none)" + + body_text = issue.body or "(empty)" + if not is_issue_visible(issue): + body_text = "(filtered)" + + labels = ", ".join(lb.name for lb in issue.labels) or "none" + + # Build PR review text if a PR is already open + review_text = "" + if pr_url: + try: + _, _, pr_number = parse_pr_url(pr_url) + reviews = get_pr_reviews(g, owner, repo, pr_number) + review_comments = get_pr_review_comments(g, owner, repo, pr_number) + review_text = _format_reviews(reviews, review_comments) + except Exception as e: + log.warning("[%s] Failed to fetch PR reviews: %s", issue_ref(owner, repo, number), e) + + # Persist metadata and branch_name before invoking Claude + update_issue_state(issue_url, { + "owner": owner, "repo": repo, "number": number, + "issue_title": issue.title, "branch_name": branch_name, + }) + + prompt = render_template( + "work.j2", + number=number, + title=issue.title, + owner=owner, + repo=repo, + labels=labels, + body=body_text, + comments_text=comments_text, + review_text=review_text, + repo_path=repo_path, + branch_name=branch_name, + pr_url=pr_url or "", + default_branch=default_branch, + ) + + log.info("[%s: %s] Invoking Claude", issue_ref(owner, repo, number), issue.title) + + # UsageLimitError/InvocationTimeoutError propagate to the orchestrator + result = invoke_claude(prompt, repo_path) + + span.set_attribute("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 work response JSON — using raw output", + issue_ref(owner, repo, number), issue.title) + summary = (result.output or "").strip()[:500] or None + + if summary: + post_comment(g, owner, repo, number, + f"{summary}{format_cost_line(result.cost_eur)}") + + # Detect PR opened by Claude and persist it + detected_pr_url = find_open_pr(g, owner, repo, branch_name) + if detected_pr_url: + if not pr_url: + _assign_reviewer(g, owner, repo, number, detected_pr_url) + update_issue_state(issue_url, {"pr_url": detected_pr_url}) + + span.set_attribute("work.pr_url", detected_pr_url or "") + log.info("[%s: %s] Work complete", issue_ref(owner, repo, number), issue.title) + + +def _assign_reviewer(g, owner: str, repo: str, number: int, pr_url: str) -> None: + """Assign the issue author as PR reviewer (skip if self).""" + try: + _, _, pr_number = parse_pr_url(pr_url) + issue_author = get_issue_author(g, owner, repo, number) + settings = get_settings() + if issue_author.lower() == settings.github_username.lower(): + log.info("[%s] Issue author is self — skipping reviewer assignment", + issue_ref(owner, repo, number)) + return + add_pr_reviewer(g, owner, repo, pr_number, issue_author) + except Exception as e: + log.warning("[%s] Failed to assign reviewer for %s: %s", + issue_ref(owner, repo, number), pr_url, e) + + +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)" diff --git a/tests/test_orchestrator.py b/tests/test_orchestrator.py index 0dd863d..86f6732 100644 --- a/tests/test_orchestrator.py +++ b/tests/test_orchestrator.py @@ -1,15 +1,14 @@ -"""Tests for clayde.orchestrator.""" +"""Tests for clayde.orchestrator — event-driven loop.""" +from datetime import datetime, timezone from unittest.mock import MagicMock, patch import pytest from clayde.orchestrator import ( - _handle_awaiting_approval, - _handle_interrupted, - _handle_new_issue, - _handle_pr_open, - _has_new_comments, + _handle_issue, + _now_utc, + _parse_timestamp, _prune_closed_issues, main, ) @@ -50,7 +49,7 @@ def test_returns_when_no_assigned_issues(self): patch("clayde.orchestrator.load_state", return_value={"issues": {}}): main() - def test_dispatches_new_issue(self): + def test_calls_handle_issue_for_each_assigned(self): issue = MagicMock() issue.html_url = "https://github.com/o/r/issues/1" with patch("clayde.orchestrator.get_settings", return_value=_mock_settings(enabled=True)), \ @@ -60,402 +59,237 @@ def test_dispatches_new_issue(self): patch("clayde.orchestrator.get_github_client"), \ patch("clayde.orchestrator.get_assigned_issues", return_value=[issue]), \ patch("clayde.orchestrator.load_state", return_value={"issues": {}}), \ - patch("clayde.orchestrator._handle_new_issue") as mock_handle: + patch("clayde.orchestrator._prune_closed_issues"), \ + patch("clayde.orchestrator._handle_issue") as mock_handle: main() mock_handle.assert_called_once() - def test_skips_done_issues(self): - issue = MagicMock() - issue.html_url = "url1" - state = {"issues": {"url1": {"status": "done"}}} - with patch("clayde.orchestrator.get_settings", return_value=_mock_settings(enabled=True)), \ - patch("clayde.orchestrator.setup_logging"), \ - patch("clayde.orchestrator.init_tracer"), \ - patch("clayde.orchestrator.is_claude_available", return_value=True), \ - patch("clayde.orchestrator.get_github_client"), \ - patch("clayde.orchestrator.get_assigned_issues", return_value=[issue]), \ - patch("clayde.orchestrator.load_state", return_value=state), \ - patch("clayde.orchestrator.plan") as mock_plan: - main() - mock_plan.run_preliminary.assert_not_called() - - def test_dispatches_awaiting_preliminary(self): - issue = MagicMock() - issue.html_url = "url1" - state = {"issues": {"url1": { - "status": "awaiting_preliminary_approval", - "owner": "o", "repo": "r", "number": 1, - "preliminary_comment_id": 100, - }}} - with patch("clayde.orchestrator.get_settings", return_value=_mock_settings(enabled=True)), \ - patch("clayde.orchestrator.setup_logging"), \ - patch("clayde.orchestrator.init_tracer"), \ - patch("clayde.orchestrator.is_claude_available", return_value=True), \ - patch("clayde.orchestrator.get_github_client"), \ - patch("clayde.orchestrator.get_assigned_issues", return_value=[issue]), \ - patch("clayde.orchestrator.load_state", return_value=state), \ - patch("clayde.orchestrator._handle_awaiting_approval") as mock_handle: - main() - mock_handle.assert_called_once() - - def test_dispatches_awaiting_plan(self): - issue = MagicMock() - issue.html_url = "url1" - state = {"issues": {"url1": { - "status": "awaiting_plan_approval", - "owner": "o", "repo": "r", "number": 1, - "plan_comment_id": 200, - }}} - with patch("clayde.orchestrator.get_settings", return_value=_mock_settings(enabled=True)), \ - patch("clayde.orchestrator.setup_logging"), \ - patch("clayde.orchestrator.init_tracer"), \ - patch("clayde.orchestrator.is_claude_available", return_value=True), \ - patch("clayde.orchestrator.get_github_client"), \ - patch("clayde.orchestrator.get_assigned_issues", return_value=[issue]), \ - patch("clayde.orchestrator.load_state", return_value=state), \ - patch("clayde.orchestrator._handle_awaiting_approval") as mock_handle: - main() - mock_handle.assert_called_once() - - def test_dispatches_pr_open(self): - issue = MagicMock() - issue.html_url = "url1" - state = {"issues": {"url1": { - "status": "pr_open", - "owner": "o", "repo": "r", "number": 1, - "pr_url": "https://github.com/o/r/pull/5", - }}} - with patch("clayde.orchestrator.get_settings", return_value=_mock_settings(enabled=True)), \ - patch("clayde.orchestrator.setup_logging"), \ - patch("clayde.orchestrator.init_tracer"), \ - patch("clayde.orchestrator.is_claude_available", return_value=True), \ - patch("clayde.orchestrator.get_github_client"), \ - patch("clayde.orchestrator.get_assigned_issues", return_value=[issue]), \ - patch("clayde.orchestrator.load_state", return_value=state), \ - patch("clayde.orchestrator._handle_pr_open") as mock_handle: - main() - mock_handle.assert_called_once() - - @pytest.mark.parametrize("transient_status", [ - "preliminary_planning", - "planning", - "implementing", - "addressing_review", - ]) - def test_recovers_transient_state_to_interrupted(self, transient_status): - """Issues stuck in transient states are converted to interrupted.""" - issue = MagicMock() - issue.html_url = "url1" - state = {"issues": {"url1": {"status": transient_status, "number": 1}}} - recovered_state = {"issues": {"url1": { - "status": "interrupted", - "interrupted_phase": transient_status, - "number": 1, - }}} - with patch("clayde.orchestrator.get_settings", return_value=_mock_settings(enabled=True)), \ - patch("clayde.orchestrator.setup_logging"), \ - patch("clayde.orchestrator.init_tracer"), \ - patch("clayde.orchestrator.is_claude_available", return_value=True), \ - patch("clayde.orchestrator.get_github_client"), \ - patch("clayde.orchestrator.get_assigned_issues", return_value=[issue]), \ - patch("clayde.orchestrator.load_state", side_effect=[state, state, recovered_state]), \ - patch("clayde.orchestrator.update_issue_state") as mock_update, \ - patch("clayde.orchestrator._handle_interrupted") as mock_handle: - main() - mock_update.assert_called_once_with( - "url1", - {"status": "interrupted", "interrupted_phase": transient_status}, - ) - mock_handle.assert_called_once() - - def test_backward_compat_awaiting_approval(self): - """Old 'awaiting_approval' status maps to 'awaiting_plan_approval'.""" + def test_main_calls_prune(self): issue = MagicMock() - issue.html_url = "url1" - state = {"issues": {"url1": { - "status": "awaiting_approval", - "owner": "o", "repo": "r", "number": 1, - "plan_comment_id": 200, - }}} + issue.html_url = "https://github.com/o/r/issues/1" with patch("clayde.orchestrator.get_settings", return_value=_mock_settings(enabled=True)), \ patch("clayde.orchestrator.setup_logging"), \ patch("clayde.orchestrator.init_tracer"), \ patch("clayde.orchestrator.is_claude_available", return_value=True), \ patch("clayde.orchestrator.get_github_client"), \ patch("clayde.orchestrator.get_assigned_issues", return_value=[issue]), \ - patch("clayde.orchestrator.load_state", return_value=state), \ - patch("clayde.orchestrator._handle_awaiting_approval") as mock_handle: + patch("clayde.orchestrator.load_state", return_value={"issues": {}}), \ + patch("clayde.orchestrator._prune_closed_issues") as mock_prune, \ + patch("clayde.orchestrator._handle_issue"): main() - mock_handle.assert_called_once() + mock_prune.assert_called_once() -class TestHandleNewIssue: +class TestHandleIssue: def test_skips_blocked_issue(self): g = MagicMock() issue = MagicMock() issue.html_url = "https://github.com/o/r/issues/1" + issue.title = "Test" with patch("clayde.orchestrator.is_blocked", return_value=True), \ patch("clayde.orchestrator.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.orchestrator.plan") as mock_plan: - _handle_new_issue(g, issue, issue.html_url) - mock_plan.run_preliminary.assert_not_called() + patch("clayde.orchestrator.work") as mock_work: + _handle_issue(g, issue, issue.html_url) + mock_work.run.assert_not_called() def test_skips_no_visible_content(self): g = MagicMock() issue = MagicMock() issue.html_url = "https://github.com/o/r/issues/1" + issue.title = "Test" with patch("clayde.orchestrator.is_blocked", return_value=False), \ patch("clayde.orchestrator.parse_issue_url", return_value=("o", "r", 1)), \ patch("clayde.orchestrator.fetch_issue_comments", return_value=[]), \ patch("clayde.orchestrator.has_visible_content", return_value=False), \ - patch("clayde.orchestrator.plan") as mock_plan: - _handle_new_issue(g, issue, issue.html_url) - mock_plan.run_preliminary.assert_not_called() + patch("clayde.orchestrator.work") as mock_work: + _handle_issue(g, issue, issue.html_url) + mock_work.run.assert_not_called() - def test_runs_preliminary_plan_when_visible(self): + def test_invokes_work_on_first_time(self): + """No last_seen_at means first-time evaluation — always invoke.""" g = MagicMock() issue = MagicMock() issue.html_url = "https://github.com/o/r/issues/1" + issue.title = "Test" with patch("clayde.orchestrator.is_blocked", return_value=False), \ patch("clayde.orchestrator.parse_issue_url", return_value=("o", "r", 1)), \ patch("clayde.orchestrator.fetch_issue_comments", return_value=[]), \ patch("clayde.orchestrator.has_visible_content", return_value=True), \ - patch("clayde.orchestrator.plan") as mock_plan: - _handle_new_issue(g, issue, issue.html_url) - mock_plan.run_preliminary.assert_called_once_with(issue.html_url) - - def test_sets_failed_on_exception(self): + patch("clayde.orchestrator.get_issue_state", return_value={}), \ + patch("clayde.orchestrator.get_new_visible_comments", return_value=[]), \ + patch("clayde.orchestrator.update_issue_state"), \ + patch("clayde.orchestrator.work") as mock_work: + _handle_issue(g, issue, issue.html_url) + mock_work.run.assert_called_once_with(issue.html_url) + + def test_invokes_work_on_new_comments(self): g = MagicMock() issue = MagicMock() issue.html_url = "https://github.com/o/r/issues/1" + issue.title = "Test" + new_comment = MagicMock() + ts = "2024-01-01T12:00:00+00:00" with patch("clayde.orchestrator.is_blocked", return_value=False), \ patch("clayde.orchestrator.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.orchestrator.fetch_issue_comments", return_value=[]), \ + patch("clayde.orchestrator.fetch_issue_comments", return_value=[new_comment]), \ patch("clayde.orchestrator.has_visible_content", return_value=True), \ - patch("clayde.orchestrator.plan") as mock_plan, \ - patch("clayde.orchestrator.update_issue_state") as mock_update: - mock_plan.run_preliminary.side_effect = RuntimeError("boom") - _handle_new_issue(g, issue, issue.html_url) - mock_update.assert_called_once_with(issue.html_url, {"status": "failed"}) - - def test_proceeds_if_blocked_check_fails(self): - """If blocked-check raises, proceed anyway (fail open).""" + patch("clayde.orchestrator.get_issue_state", return_value={"last_seen_at": ts}), \ + patch("clayde.orchestrator.get_new_visible_comments", return_value=[new_comment]), \ + patch("clayde.orchestrator.update_issue_state"), \ + patch("clayde.orchestrator.work") as mock_work: + _handle_issue(g, issue, issue.html_url) + mock_work.run.assert_called_once_with(issue.html_url) + + def test_skips_when_no_new_activity(self): g = MagicMock() issue = MagicMock() issue.html_url = "https://github.com/o/r/issues/1" - with patch("clayde.orchestrator.is_blocked", side_effect=Exception("API error")), \ + issue.title = "Test" + ts = "2024-01-01T12:00:00+00:00" + with patch("clayde.orchestrator.is_blocked", return_value=False), \ patch("clayde.orchestrator.parse_issue_url", return_value=("o", "r", 1)), \ patch("clayde.orchestrator.fetch_issue_comments", return_value=[]), \ patch("clayde.orchestrator.has_visible_content", return_value=True), \ - patch("clayde.orchestrator.plan") as mock_plan: - _handle_new_issue(g, issue, issue.html_url) - mock_plan.run_preliminary.assert_called_once() - - -class TestHandleAwaitingPreliminary: - def test_does_nothing_when_not_approved_and_no_new_comments(self): + patch("clayde.orchestrator.get_issue_state", return_value={"last_seen_at": ts}), \ + patch("clayde.orchestrator.get_new_visible_comments", return_value=[]), \ + patch("clayde.orchestrator.work") as mock_work: + _handle_issue(g, issue, issue.html_url) + mock_work.run.assert_not_called() + + def test_invokes_work_when_in_progress(self): + """Crash recovery: in_progress=True triggers invocation even with no new activity.""" g = MagicMock() - entry = {"owner": "o", "repo": "r", "number": 1, "preliminary_comment_id": 100} - with patch("clayde.orchestrator._has_new_comments", return_value=False), \ - patch("clayde.orchestrator.is_plan_approved", return_value=False), \ - patch("clayde.orchestrator.plan") as mock_plan: - _handle_awaiting_approval(g, "url", entry, phase="preliminary") - mock_plan.run_thorough.assert_not_called() - mock_plan.run_update.assert_not_called() - - def test_runs_thorough_when_approved(self): + issue = MagicMock() + issue.html_url = "https://github.com/o/r/issues/1" + issue.title = "Test" + ts = "2024-01-01T12:00:00+00:00" + with patch("clayde.orchestrator.is_blocked", return_value=False), \ + patch("clayde.orchestrator.parse_issue_url", return_value=("o", "r", 1)), \ + patch("clayde.orchestrator.fetch_issue_comments", return_value=[]), \ + patch("clayde.orchestrator.has_visible_content", return_value=True), \ + patch("clayde.orchestrator.get_issue_state", + return_value={"last_seen_at": ts, "in_progress": True}), \ + patch("clayde.orchestrator.get_new_visible_comments", return_value=[]), \ + patch("clayde.orchestrator.update_issue_state"), \ + patch("clayde.orchestrator.work") as mock_work: + _handle_issue(g, issue, issue.html_url) + mock_work.run.assert_called_once_with(issue.html_url) + + def test_sets_in_progress_before_invoke_and_clears_after(self): g = MagicMock() - entry = {"owner": "o", "repo": "r", "number": 1, "preliminary_comment_id": 100} - with patch("clayde.orchestrator._has_new_comments", return_value=False), \ - patch("clayde.orchestrator.is_plan_approved", return_value=True), \ - patch("clayde.orchestrator.plan") as mock_plan: - _handle_awaiting_approval(g, "url", entry, phase="preliminary") - mock_plan.run_thorough.assert_called_once_with("url") - - def test_runs_update_when_new_comments(self): + issue = MagicMock() + issue.html_url = "https://github.com/o/r/issues/1" + issue.title = "Test" + calls = [] + with patch("clayde.orchestrator.is_blocked", return_value=False), \ + patch("clayde.orchestrator.parse_issue_url", return_value=("o", "r", 1)), \ + patch("clayde.orchestrator.fetch_issue_comments", return_value=[]), \ + patch("clayde.orchestrator.has_visible_content", return_value=True), \ + patch("clayde.orchestrator.get_issue_state", return_value={}), \ + patch("clayde.orchestrator.get_new_visible_comments", return_value=[]), \ + patch("clayde.orchestrator.update_issue_state", + side_effect=lambda url, upd: calls.append(upd)), \ + patch("clayde.orchestrator.work"): + _handle_issue(g, issue, issue.html_url) + + # First update sets in_progress=True + assert calls[0] == {"in_progress": True} + # Last update clears in_progress and sets last_seen_at + last = calls[-1] + assert last.get("in_progress") is False + assert "last_seen_at" in last + + def test_leaves_in_progress_on_usage_limit(self): + from clayde.claude import UsageLimitError g = MagicMock() - entry = {"owner": "o", "repo": "r", "number": 1, "preliminary_comment_id": 100} - with patch("clayde.orchestrator._has_new_comments", return_value=True), \ - patch("clayde.orchestrator.plan") as mock_plan: - _handle_awaiting_approval(g, "url", entry, phase="preliminary") - mock_plan.run_update.assert_called_once_with("url", "preliminary") + issue = MagicMock() + issue.html_url = "https://github.com/o/r/issues/1" + issue.title = "Test" + calls = [] + with patch("clayde.orchestrator.is_blocked", return_value=False), \ + patch("clayde.orchestrator.parse_issue_url", return_value=("o", "r", 1)), \ + patch("clayde.orchestrator.fetch_issue_comments", return_value=[]), \ + patch("clayde.orchestrator.has_visible_content", return_value=True), \ + patch("clayde.orchestrator.get_issue_state", return_value={}), \ + patch("clayde.orchestrator.get_new_visible_comments", return_value=[]), \ + patch("clayde.orchestrator.update_issue_state", + side_effect=lambda url, upd: calls.append(upd)), \ + patch("clayde.orchestrator.work") as mock_work: + mock_work.run.side_effect = UsageLimitError("limit", cost_eur=0.0) + _handle_issue(g, issue, issue.html_url) + + # First call sets in_progress=True, nothing clears it + assert calls[0] == {"in_progress": True} + # No subsequent call should set in_progress=False + assert not any(c.get("in_progress") is False for c in calls) - def test_runs_implement_directly_when_small_and_approved(self): - g = MagicMock() - entry = {"owner": "o", "repo": "r", "number": 1, "preliminary_comment_id": 100, "size": "small"} - with patch("clayde.orchestrator._has_new_comments", return_value=False), \ - patch("clayde.orchestrator.is_plan_approved", return_value=True), \ - patch("clayde.orchestrator.plan") as mock_plan, \ - patch("clayde.orchestrator.implement") as mock_impl: - _handle_awaiting_approval(g, "url", entry, phase="preliminary") - mock_plan.run_thorough.assert_not_called() - mock_impl.run.assert_called_once_with("url") - - def test_runs_thorough_when_large_and_approved(self): - g = MagicMock() - entry = {"owner": "o", "repo": "r", "number": 1, "preliminary_comment_id": 100, "size": "large"} - with patch("clayde.orchestrator._has_new_comments", return_value=False), \ - patch("clayde.orchestrator.is_plan_approved", return_value=True), \ - patch("clayde.orchestrator.plan") as mock_plan, \ - patch("clayde.orchestrator.implement") as mock_impl: - _handle_awaiting_approval(g, "url", entry, phase="preliminary") - mock_plan.run_thorough.assert_called_once_with("url") - mock_impl.run.assert_not_called() - - def test_defaults_to_large_when_size_absent(self): - """In-flight issues without size in state default to large behavior.""" - g = MagicMock() - entry = {"owner": "o", "repo": "r", "number": 1, "preliminary_comment_id": 100} - with patch("clayde.orchestrator._has_new_comments", return_value=False), \ - patch("clayde.orchestrator.is_plan_approved", return_value=True), \ - patch("clayde.orchestrator.plan") as mock_plan, \ - patch("clayde.orchestrator.implement") as mock_impl: - _handle_awaiting_approval(g, "url", entry, phase="preliminary") - mock_plan.run_thorough.assert_called_once_with("url") - mock_impl.run.assert_not_called() - - def test_sets_failed_on_thorough_exception(self): + def test_proceeds_if_blocked_check_fails(self): g = MagicMock() - entry = {"owner": "o", "repo": "r", "number": 1, "preliminary_comment_id": 100} - with patch("clayde.orchestrator._has_new_comments", return_value=False), \ - patch("clayde.orchestrator.is_plan_approved", return_value=True), \ - patch("clayde.orchestrator.plan") as mock_plan, \ - patch("clayde.orchestrator.update_issue_state") as mock_update: - mock_plan.run_thorough.side_effect = RuntimeError("boom") - _handle_awaiting_approval(g, "url", entry, phase="preliminary") - mock_update.assert_called_once_with("url", {"status": "failed"}) - - def test_marks_failed_if_no_preliminary_comment_id(self): + issue = MagicMock() + issue.html_url = "https://github.com/o/r/issues/1" + issue.title = "Test" + with patch("clayde.orchestrator.is_blocked", side_effect=Exception("API error")), \ + patch("clayde.orchestrator.parse_issue_url", return_value=("o", "r", 1)), \ + patch("clayde.orchestrator.fetch_issue_comments", return_value=[]), \ + patch("clayde.orchestrator.has_visible_content", return_value=True), \ + patch("clayde.orchestrator.get_issue_state", return_value={}), \ + patch("clayde.orchestrator.get_new_visible_comments", return_value=[]), \ + patch("clayde.orchestrator.update_issue_state"), \ + patch("clayde.orchestrator.work") as mock_work: + _handle_issue(g, issue, issue.html_url) + mock_work.run.assert_called_once() + + def test_clears_in_progress_on_unexpected_error(self): g = MagicMock() - entry = {"owner": "o", "repo": "r", "number": 1} - with patch("clayde.orchestrator.update_issue_state") as mock_update: - _handle_awaiting_approval(g, "url", entry, phase="preliminary") - mock_update.assert_called_once_with("url", {"status": "failed"}) + issue = MagicMock() + issue.html_url = "https://github.com/o/r/issues/1" + issue.title = "Test" + calls = [] + with patch("clayde.orchestrator.is_blocked", return_value=False), \ + patch("clayde.orchestrator.parse_issue_url", return_value=("o", "r", 1)), \ + patch("clayde.orchestrator.fetch_issue_comments", return_value=[]), \ + patch("clayde.orchestrator.has_visible_content", return_value=True), \ + patch("clayde.orchestrator.get_issue_state", return_value={}), \ + patch("clayde.orchestrator.get_new_visible_comments", return_value=[]), \ + patch("clayde.orchestrator.update_issue_state", + side_effect=lambda url, upd: calls.append(upd)), \ + patch("clayde.orchestrator.work") as mock_work: + mock_work.run.side_effect = RuntimeError("boom") + _handle_issue(g, issue, issue.html_url) + # in_progress should be cleared to False after an unexpected error + assert any(c.get("in_progress") is False for c in calls) -class TestHandleAwaitingPlan: - def test_does_nothing_when_not_approved_and_no_new_comments(self): - g = MagicMock() - entry = {"owner": "o", "repo": "r", "number": 1, "plan_comment_id": 200} - with patch("clayde.orchestrator._has_new_comments", return_value=False), \ - patch("clayde.orchestrator.is_plan_approved", return_value=False), \ - patch("clayde.orchestrator.implement") as mock_impl: - _handle_awaiting_approval(g, "url", entry, phase="thorough") - mock_impl.run.assert_not_called() - - def test_runs_implement_when_approved(self): - g = MagicMock() - entry = {"owner": "o", "repo": "r", "number": 1, "plan_comment_id": 200} - with patch("clayde.orchestrator._has_new_comments", return_value=False), \ - patch("clayde.orchestrator.is_plan_approved", return_value=True), \ - patch("clayde.orchestrator.implement") as mock_impl: - _handle_awaiting_approval(g, "url", entry, phase="thorough") - mock_impl.run.assert_called_once_with("url") - - def test_runs_update_when_new_comments(self): - g = MagicMock() - entry = {"owner": "o", "repo": "r", "number": 1, "plan_comment_id": 200} - with patch("clayde.orchestrator._has_new_comments", return_value=True), \ - patch("clayde.orchestrator.plan") as mock_plan: - _handle_awaiting_approval(g, "url", entry, phase="thorough") - mock_plan.run_update.assert_called_once_with("url", "thorough") - def test_sets_failed_on_exception(self): - g = MagicMock() - entry = {"owner": "o", "repo": "r", "number": 1, "plan_comment_id": 200} - with patch("clayde.orchestrator._has_new_comments", return_value=False), \ - patch("clayde.orchestrator.is_plan_approved", return_value=True), \ - patch("clayde.orchestrator.implement") as mock_impl, \ - patch("clayde.orchestrator.update_issue_state") as mock_update: - mock_impl.run.side_effect = RuntimeError("boom") - _handle_awaiting_approval(g, "url", entry, phase="thorough") - mock_update.assert_called_once_with("url", {"status": "failed"}) - - def test_marks_failed_if_no_plan_comment_id(self): - g = MagicMock() - entry = {"owner": "o", "repo": "r", "number": 1} - with patch("clayde.orchestrator.update_issue_state") as mock_update: - _handle_awaiting_approval(g, "url", entry, phase="thorough") - mock_update.assert_called_once_with("url", {"status": "failed"}) +class TestParseTimestamp: + def test_parses_utc_z_suffix(self): + ts = "2024-01-15T10:30:00Z" + result = _parse_timestamp(ts) + assert result is not None + assert result.tzinfo is not None + assert result.year == 2024 + def test_parses_offset_format(self): + ts = "2024-01-15T10:30:00+00:00" + result = _parse_timestamp(ts) + assert result is not None -class TestHandlePrOpen: - def test_runs_review(self): - g = MagicMock() - entry = {"number": 1, "pr_url": "https://github.com/o/r/pull/5"} - with patch("clayde.orchestrator.review") as mock_review: - _handle_pr_open(g, "url", entry) - mock_review.run.assert_called_once_with("url") + def test_returns_none_for_none(self): + assert _parse_timestamp(None) is None - def test_sets_failed_on_exception(self): - g = MagicMock() - entry = {"number": 1} - with patch("clayde.orchestrator.review") as mock_review, \ - patch("clayde.orchestrator.update_issue_state") as mock_update: - mock_review.run.side_effect = RuntimeError("boom") - _handle_pr_open(g, "url", entry) - mock_update.assert_called_once_with("url", {"status": "failed"}) - - -class TestHandleInterrupted: - def test_retries_preliminary_planning(self): - entry = {"interrupted_phase": "preliminary_planning"} - with patch("clayde.orchestrator.plan") as mock_plan: - _handle_interrupted("url", entry) - mock_plan.run_preliminary.assert_called_once_with("url") - - def test_retries_planning(self): - entry = {"interrupted_phase": "planning"} - with patch("clayde.orchestrator.plan") as mock_plan: - _handle_interrupted("url", entry) - mock_plan.run_thorough.assert_called_once_with("url") - - def test_retries_implementing(self): - entry = {"interrupted_phase": "implementing"} - with patch("clayde.orchestrator.implement") as mock_impl: - _handle_interrupted("url", entry) - mock_impl.run.assert_called_once_with("url") - - def test_retries_addressing_review(self): - entry = {"interrupted_phase": "addressing_review"} - with patch("clayde.orchestrator.review") as mock_review: - _handle_interrupted("url", entry) - mock_review.run.assert_called_once_with("url") - - def test_skips_unknown_phase(self): - entry = {"interrupted_phase": "unknown"} - with patch("clayde.orchestrator.plan") as mock_plan, \ - patch("clayde.orchestrator.implement") as mock_impl: - _handle_interrupted("url", entry) - mock_plan.run_preliminary.assert_not_called() - mock_plan.run_thorough.assert_not_called() - mock_impl.run.assert_not_called() - - def test_stays_interrupted_on_error(self): - entry = {"interrupted_phase": "preliminary_planning"} - with patch("clayde.orchestrator.plan") as mock_plan, \ - patch("clayde.orchestrator.update_issue_state") as mock_update: - mock_plan.run_preliminary.side_effect = RuntimeError("boom") - _handle_interrupted("url", entry) - mock_update.assert_called_once_with("url", {"status": "interrupted"}) - - -class TestHasNewComments: - def test_detects_new_visible_comments(self): - g = MagicMock() - comment = MagicMock() - with patch("clayde.orchestrator.fetch_issue_comments", return_value=[comment]), \ - patch("clayde.orchestrator.get_new_visible_comments", return_value=[comment]): - entry = {"last_seen_comment_id": 100} - assert _has_new_comments(g, "o", "r", 1, entry) is True + def test_returns_none_for_empty(self): + assert _parse_timestamp("") is None - def test_no_new_comments(self): - g = MagicMock() - comment = MagicMock() - with patch("clayde.orchestrator.fetch_issue_comments", return_value=[comment]), \ - patch("clayde.orchestrator.get_new_visible_comments", return_value=[]): - entry = {"last_seen_comment_id": 100} - assert _has_new_comments(g, "o", "r", 1, entry) is False + def test_returns_none_for_invalid(self): + assert _parse_timestamp("not-a-timestamp") is None + + +class TestNowUtc: + def test_returns_iso_string(self): + ts = _now_utc() + assert isinstance(ts, str) + parsed = datetime.fromisoformat(ts) + assert parsed.tzinfo is not None class TestPruneClosedIssues: @@ -486,7 +320,7 @@ def test_keeps_open_issue(self): def test_skips_entry_missing_fields(self): g = MagicMock() - issues_state = {"url1": {"status": "done"}} # no owner/repo/number + issues_state = {"url1": {"last_seen_at": "2024-01-01T00:00:00Z"}} with patch("clayde.orchestrator.fetch_issue") as mock_fetch, \ patch("clayde.orchestrator.save_state") as mock_save: _prune_closed_issues(g, issues_state) @@ -515,29 +349,11 @@ def test_prunes_multiple_closed_in_one_save(self): patch("clayde.orchestrator.load_state", return_value={"issues": dict(issues_state)}), \ patch("clayde.orchestrator.save_state") as mock_save: _prune_closed_issues(g, issues_state) - # Only one save call (batched) assert mock_save.call_count == 1 saved = mock_save.call_args[0][0] assert url1 not in saved["issues"] assert url2 not in saved["issues"] - def test_main_calls_prune(self): - """main() calls _prune_closed_issues before processing issues.""" - issue = MagicMock() - issue.html_url = "https://github.com/o/r/issues/1" - state = {"issues": {}} - with patch("clayde.orchestrator.get_settings", return_value=_mock_settings(enabled=True)), \ - patch("clayde.orchestrator.setup_logging"), \ - patch("clayde.orchestrator.init_tracer"), \ - patch("clayde.orchestrator.is_claude_available", return_value=True), \ - patch("clayde.orchestrator.get_github_client"), \ - patch("clayde.orchestrator.get_assigned_issues", return_value=[issue]), \ - patch("clayde.orchestrator.load_state", return_value=state), \ - patch("clayde.orchestrator._prune_closed_issues") as mock_prune, \ - patch("clayde.orchestrator._handle_new_issue"): - main() - mock_prune.assert_called_once() - def test_run_loop_without_pebble_uses_legacy_path(monkeypatch): """When pebble_enabled is False, run_loop must use the existing sync path.""" @@ -545,7 +361,7 @@ def test_run_loop_without_pebble_uses_legacy_path(monkeypatch): calls = [] monkeypatch.setattr(orchestrator, "main", lambda: calls.append("tick")) - monkeypatch.setattr(orchestrator, "_shutdown", True) # exit after first iteration + monkeypatch.setattr(orchestrator, "_shutdown", True) monkeypatch.setattr(orchestrator, "setup_logging", lambda: None) class _S: @@ -554,8 +370,6 @@ class _S: monkeypatch.setattr(orchestrator, "get_settings", lambda: _S()) orchestrator.run_loop() - # _shutdown=True from start means main() never runs; that's fine — the - # important assertion is no exception and no asyncio.run call. def test_run_loop_with_pebble_invokes_async_entry(monkeypatch): diff --git a/tests/test_responses.py b/tests/test_responses.py index 4871918..fc94c17 100644 --- a/tests/test_responses.py +++ b/tests/test_responses.py @@ -3,11 +3,7 @@ import pytest from clayde.responses import ( - AddressReviewResponse, - ImplementResponse, - PreliminaryPlanResponse, - ThoroughPlanResponse, - UpdatePlanResponse, + WorkResponse, _extract_json, parse_response, ) @@ -15,119 +11,80 @@ class TestExtractJson: def test_strips_json_fences(self): - text = "```json\n{\"plan\": \"hello\"}\n```" - assert _extract_json(text) == '{"plan": "hello"}' + text = "```json\n{\"summary\": \"hello\"}\n```" + assert _extract_json(text) == '{"summary": "hello"}' def test_strips_plain_fences(self): - text = "```\n{\"plan\": \"hello\"}\n```" - assert _extract_json(text) == '{"plan": "hello"}' + text = "```\n{\"summary\": \"hello\"}\n```" + assert _extract_json(text) == '{"summary": "hello"}' def test_no_fences_unchanged(self): - text = '{"plan": "hello"}' + text = '{"summary": "hello"}' assert _extract_json(text) == text def test_strips_surrounding_whitespace(self): - text = ' {"plan": "hello"} ' - assert _extract_json(text) == '{"plan": "hello"}' + text = ' {"summary": "hello"} ' + assert _extract_json(text) == '{"summary": "hello"}' def test_partial_fence_unchanged(self): # Only opening fence — should not strip - text = "```json\n{\"plan\": \"hello\"}" + text = "```json\n{\"summary\": \"hello\"}" result = _extract_json(text) - # no closing fence so brace-matching finds the JSON object - assert "plan" in result + assert "summary" in result def test_extracts_json_with_preamble(self): - text = 'Here is my plan.\n\n```json\n{"plan": "hello"}\n```' - assert _extract_json(text) == '{"plan": "hello"}' + text = 'Here is my plan.\n\n```json\n{"summary": "hello"}\n```' + assert _extract_json(text) == '{"summary": "hello"}' def test_extracts_json_object_without_fences(self): - text = 'Now I have enough context.\n\n{"plan": "hello"}' - assert _extract_json(text) == '{"plan": "hello"}' + text = 'Now I have enough context.\n\n{"summary": "hello"}' + assert _extract_json(text) == '{"summary": "hello"}' def test_handles_nested_braces_in_json(self): - text = 'Some text\n{"plan": "use {braces} here"}' + text = 'Some text\n{"summary": "use {braces} here"}' result = _extract_json(text) - assert '"plan"' in result + assert '"summary"' in result assert "{braces}" in result def test_handles_escaped_quotes(self): - text = r'Preamble {"plan": "say \"hello\""}' + text = r'Preamble {"summary": "say \"hello\""}' result = _extract_json(text) - assert '"plan"' in result + assert '"summary"' in result class TestParseResponse: - def test_parses_preliminary_plan(self): - text = '{"plan": "My plan here", "size": "small", "branch_name": "clayde/issue-1-fix"}' - result = parse_response(text, PreliminaryPlanResponse) - assert isinstance(result, PreliminaryPlanResponse) - assert result.plan == "My plan here" - assert result.size == "small" - assert result.branch_name == "clayde/issue-1-fix" - - def test_parses_preliminary_plan_large(self): - text = '{"plan": "Big plan", "size": "large", "branch_name": "clayde/issue-2-big-feature"}' - result = parse_response(text, PreliminaryPlanResponse) - assert result.size == "large" - - def test_parses_thorough_plan(self): - text = '{"plan": "Thorough plan"}' - result = parse_response(text, ThoroughPlanResponse) - assert isinstance(result, ThoroughPlanResponse) - assert result.plan == "Thorough plan" - - def test_parses_update_plan(self): - text = '{"summary": "Changed X", "updated_plan": "Updated plan"}' - result = parse_response(text, UpdatePlanResponse) - assert isinstance(result, UpdatePlanResponse) - assert result.summary == "Changed X" - assert result.updated_plan == "Updated plan" - - def test_parses_implement_response(self): + def test_parses_work_response(self): text = '{"summary": "Implemented the feature"}' - result = parse_response(text, ImplementResponse) - assert isinstance(result, ImplementResponse) + result = parse_response(text, WorkResponse) + assert isinstance(result, WorkResponse) assert result.summary == "Implemented the feature" - def test_parses_address_review_response(self): - text = '{"summary": "Fixed the typo"}' - result = parse_response(text, AddressReviewResponse) - assert isinstance(result, AddressReviewResponse) - assert result.summary == "Fixed the typo" - def test_strips_code_fences_before_parsing(self): - text = '```json\n{"plan": "My plan", "size": "small", "branch_name": "clayde/issue-1-fix"}\n```' - result = parse_response(text, PreliminaryPlanResponse) - assert result.plan == "My plan" + text = '```json\n{"summary": "Done"}\n```' + result = parse_response(text, WorkResponse) + assert result.summary == "Done" def test_invalid_json_raises_value_error(self): with pytest.raises(ValueError, match="not valid JSON"): - parse_response("this is not json", PreliminaryPlanResponse) + parse_response("this is not json", WorkResponse) def test_missing_required_field_raises_value_error(self): - # PreliminaryPlanResponse requires plan, size, and branch_name - text = '{"plan": "only plan, no size or branch_name"}' - with pytest.raises(ValueError, match="failed validation"): - parse_response(text, PreliminaryPlanResponse) - - def test_wrong_type_raises_value_error(self): - text = '{"plan": 123}' # plan should be a string + text = '{"other_field": "value"}' with pytest.raises(ValueError, match="failed validation"): - parse_response(text, PreliminaryPlanResponse) + parse_response(text, WorkResponse) def test_empty_string_raises_value_error(self): with pytest.raises(ValueError): - parse_response("", PreliminaryPlanResponse) + parse_response("", WorkResponse) def test_extra_fields_are_ignored(self): - text = '{"plan": "My plan", "size": "small", "branch_name": "clayde/issue-1-fix", "extra_field": "ignored"}' - result = parse_response(text, PreliminaryPlanResponse) - assert result.plan == "My plan" + text = '{"summary": "Done", "extra_field": "ignored"}' + result = parse_response(text, WorkResponse) + assert result.summary == "Done" - def test_multiline_plan_preserved(self): + def test_multiline_summary_preserved(self): import json - plan_content = "## Plan\n\nStep 1\nStep 2" - text = json.dumps({"plan": plan_content, "size": "large", "branch_name": "clayde/issue-1-fix"}) - result = parse_response(text, PreliminaryPlanResponse) - assert result.plan == plan_content + content = "## Summary\n\nStep 1\nStep 2" + text = json.dumps({"summary": content}) + result = parse_response(text, WorkResponse) + assert result.summary == content diff --git a/tests/test_safety.py b/tests/test_safety.py index aa4a5a8..8c474df 100644 --- a/tests/test_safety.py +++ b/tests/test_safety.py @@ -1,14 +1,15 @@ """Tests for clayde.safety.""" +from datetime import datetime, timezone from unittest.mock import MagicMock, patch from clayde.safety import ( _has_whitelisted_reaction, filter_comments, + get_new_visible_comments, has_visible_content, is_comment_visible, is_issue_visible, - is_plan_approved, ) @@ -19,16 +20,19 @@ def _make_reaction(content, login): return r -def _mock_settings(users): +def _mock_settings(users, github_username="ClaydeCode"): s = MagicMock() s.whitelisted_users_list = users + s.github_username = github_username return s -def _make_comment(login, reactions=None): +def _make_comment(login, reactions=None, created_at=None): c = MagicMock() c.user.login = login c.get_reactions.return_value = reactions or [] + if created_at is not None: + c.created_at = created_at return c @@ -134,22 +138,55 @@ def test_false_when_no_comments_and_invisible_issue(self): assert has_visible_content(issue, []) is False -class TestIsPlanApproved: - def test_approved_with_thumbsup(self): - g = MagicMock() - comment = MagicMock() - comment.get_reactions.return_value = [_make_reaction("+1", "alice")] - g.get_repo.return_value.get_issue.return_value.get_comment.return_value = comment - with patch("clayde.safety.get_settings", return_value=_mock_settings(["alice"])): - assert is_plan_approved(g, "owner", "repo", 1, 100) is True - - def test_not_approved_wrong_reaction(self): - g = MagicMock() - comment = MagicMock() - comment.get_reactions.return_value = [_make_reaction("heart", "alice")] - g.get_repo.return_value.get_issue.return_value.get_comment.return_value = comment - with patch("clayde.safety.get_settings", return_value=_mock_settings(["alice"])): - assert is_plan_approved(g, "owner", "repo", 1, 100) is False +class TestGetNewVisibleComments: + def _make_ts(self, year=2024, month=1, day=1, hour=12): + return datetime(year, month, day, hour, tzinfo=timezone.utc) + + def test_returns_all_non_clayde_when_last_seen_none(self): + """No last_seen_at (first time) returns all visible non-Clayde comments.""" + c1 = _make_comment("alice", created_at=self._make_ts(hour=10)) + c2 = _make_comment("bob", created_at=self._make_ts(hour=11)) + with patch("clayde.safety.get_settings", + return_value=_mock_settings(["alice", "bob"], github_username="ClaydeCode")): + result = get_new_visible_comments([c1, c2], None) + assert c1 in result + assert c2 in result + + def test_excludes_clayde_comments_when_last_seen_none(self): + clayde_c = _make_comment("ClaydeCode", created_at=self._make_ts()) + visible_c = _make_comment("alice", created_at=self._make_ts()) + with patch("clayde.safety.get_settings", + return_value=_mock_settings(["alice", "ClaydeCode"], github_username="ClaydeCode")): + result = get_new_visible_comments([clayde_c, visible_c], None) + assert clayde_c not in result + assert visible_c in result + + def test_returns_comments_newer_than_last_seen(self): + last_seen = self._make_ts(hour=10) + old_c = _make_comment("alice", created_at=self._make_ts(hour=9)) + new_c = _make_comment("alice", created_at=self._make_ts(hour=11)) + with patch("clayde.safety.get_settings", + return_value=_mock_settings(["alice"], github_username="ClaydeCode")): + result = get_new_visible_comments([old_c, new_c], last_seen) + assert old_c not in result + assert new_c in result + + def test_excludes_invisible_comments(self): + last_seen = self._make_ts(hour=10) + invisible = _make_comment("mallory", created_at=self._make_ts(hour=11)) + # mallory not whitelisted and no +1 reaction → invisible + with patch("clayde.safety.get_settings", + return_value=_mock_settings(["alice"], github_username="ClaydeCode")): + result = get_new_visible_comments([invisible], last_seen) + assert invisible not in result + + def test_excludes_clayde_own_comments_with_last_seen(self): + last_seen = self._make_ts(hour=10) + clayde_c = _make_comment("ClaydeCode", created_at=self._make_ts(hour=11)) + with patch("clayde.safety.get_settings", + return_value=_mock_settings(["ClaydeCode"], github_username="ClaydeCode")): + result = get_new_visible_comments([clayde_c], last_seen) + assert clayde_c not in result class TestHasWhitelistedReaction: diff --git a/tests/test_state.py b/tests/test_state.py index da1e277..d91479a 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -4,8 +4,6 @@ from pathlib import Path from unittest.mock import patch -import pytest - import clayde.state as state_mod @@ -17,7 +15,7 @@ def test_returns_empty_when_file_missing(self, tmp_path): def test_loads_existing_state(self, tmp_path): sf = tmp_path / "state.json" - data = {"issues": {"url1": {"status": "done"}}} + data = {"issues": {"url1": {"last_seen_at": "2024-01-01T00:00:00Z"}}} sf.write_text(json.dumps(data)) with patch.object(state_mod, "_STATE_FILE", sf): assert state_mod.load_state() == data @@ -27,7 +25,7 @@ class TestSaveState: def test_writes_json(self, tmp_path): sf = tmp_path / "state.json" with patch.object(state_mod, "_STATE_FILE", sf): - data = {"issues": {"url": {"status": "planning"}}} + data = {"issues": {"url": {"last_seen_at": "2024-01-01T00:00:00Z"}}} state_mod.save_state(data) assert json.loads(sf.read_text()) == data @@ -41,7 +39,7 @@ def test_returns_empty_for_unknown_url(self, tmp_path): def test_returns_entry_for_known_url(self, tmp_path): sf = tmp_path / "state.json" - entry = {"status": "done", "pr_url": "https://example.com"} + entry = {"last_seen_at": "2024-01-01T00:00:00Z", "pr_url": "https://example.com"} sf.write_text(json.dumps({"issues": {"url1": entry}})) with patch.object(state_mod, "_STATE_FILE", sf): assert state_mod.get_issue_state("url1") == entry @@ -52,86 +50,25 @@ def test_creates_new_entry(self, tmp_path): sf = tmp_path / "state.json" sf.write_text(json.dumps({"issues": {}})) with patch.object(state_mod, "_STATE_FILE", sf): - state_mod.update_issue_state("url1", {"status": "planning", "owner": "o"}) + state_mod.update_issue_state("url1", { + "last_seen_at": "2024-01-01T00:00:00Z", "owner": "o", + }) result = json.loads(sf.read_text()) - assert result["issues"]["url1"]["status"] == "planning" + assert result["issues"]["url1"]["last_seen_at"] == "2024-01-01T00:00:00Z" assert result["issues"]["url1"]["owner"] == "o" def test_merges_updates(self, tmp_path): sf = tmp_path / "state.json" - sf.write_text(json.dumps({"issues": {"url1": {"status": "planning", "owner": "o"}}})) + sf.write_text(json.dumps({"issues": {"url1": { + "last_seen_at": "2024-01-01T00:00:00Z", "owner": "o", + }}})) with patch.object(state_mod, "_STATE_FILE", sf): - state_mod.update_issue_state("url1", {"status": "done", "pr_url": "pr"}) + state_mod.update_issue_state("url1", { + "last_seen_at": "2024-06-01T00:00:00Z", "pr_url": "pr", + }) result = json.loads(sf.read_text()) - assert result["issues"]["url1"] == {"status": "done", "owner": "o", "pr_url": "pr"} - - -class TestAccumulateCost: - def test_accumulates_on_new_issue(self, tmp_path): - sf = tmp_path / "state.json" - sf.write_text(json.dumps({"issues": {}})) - with patch.object(state_mod, "_STATE_FILE", sf): - state_mod.accumulate_cost("url1", 1.50) - result = json.loads(sf.read_text()) - assert result["issues"]["url1"]["accumulated_cost_eur"] == pytest.approx(1.50) - - def test_accumulates_on_existing_issue(self, tmp_path): - sf = tmp_path / "state.json" - sf.write_text(json.dumps({"issues": {"url1": {"status": "interrupted"}}})) - with patch.object(state_mod, "_STATE_FILE", sf): - state_mod.accumulate_cost("url1", 1.00) - state_mod.accumulate_cost("url1", 0.50) - result = json.loads(sf.read_text()) - assert result["issues"]["url1"]["accumulated_cost_eur"] == pytest.approx(1.50) - # Existing fields are preserved - assert result["issues"]["url1"]["status"] == "interrupted" - - def test_accumulates_multiple_times(self, tmp_path): - sf = tmp_path / "state.json" - sf.write_text(json.dumps({"issues": {}})) - with patch.object(state_mod, "_STATE_FILE", sf): - state_mod.accumulate_cost("url1", 0.10) - state_mod.accumulate_cost("url1", 0.20) - state_mod.accumulate_cost("url1", 0.30) - result = json.loads(sf.read_text()) - assert result["issues"]["url1"]["accumulated_cost_eur"] == pytest.approx(0.60) - - -class TestPopAccumulatedCost: - def test_returns_and_resets_cost(self, tmp_path): - sf = tmp_path / "state.json" - sf.write_text(json.dumps({"issues": {"url1": {"accumulated_cost_eur": 2.50, "status": "ok"}}})) - with patch.object(state_mod, "_STATE_FILE", sf): - cost = state_mod.pop_accumulated_cost("url1") - assert cost == pytest.approx(2.50) - result = json.loads(sf.read_text()) - assert "accumulated_cost_eur" not in result["issues"]["url1"] - # Other fields preserved - assert result["issues"]["url1"]["status"] == "ok" - - def test_returns_zero_when_no_cost(self, tmp_path): - sf = tmp_path / "state.json" - sf.write_text(json.dumps({"issues": {"url1": {"status": "ok"}}})) - with patch.object(state_mod, "_STATE_FILE", sf): - cost = state_mod.pop_accumulated_cost("url1") - assert cost == 0.0 - - def test_returns_zero_for_unknown_issue(self, tmp_path): - sf = tmp_path / "state.json" - sf.write_text(json.dumps({"issues": {}})) - with patch.object(state_mod, "_STATE_FILE", sf): - cost = state_mod.pop_accumulated_cost("unknown") - assert cost == 0.0 - - def test_accumulate_then_pop(self, tmp_path): - """Full cycle: accumulate across interruptions, then pop the total.""" - sf = tmp_path / "state.json" - sf.write_text(json.dumps({"issues": {}})) - with patch.object(state_mod, "_STATE_FILE", sf): - state_mod.accumulate_cost("url1", 1.00) - state_mod.accumulate_cost("url1", 2.00) - total = state_mod.pop_accumulated_cost("url1") - assert total == pytest.approx(3.00) - # After pop, accumulator is reset - with patch.object(state_mod, "_STATE_FILE", sf): - assert state_mod.pop_accumulated_cost("url1") == 0.0 + assert result["issues"]["url1"] == { + "last_seen_at": "2024-06-01T00:00:00Z", + "owner": "o", + "pr_url": "pr", + } diff --git a/tests/test_tasks_implement.py b/tests/test_tasks_implement.py deleted file mode 100644 index e061819..0000000 --- a/tests/test_tasks_implement.py +++ /dev/null @@ -1,527 +0,0 @@ -"""Tests for clayde.tasks.implement.""" - -from pathlib import Path -from unittest.mock import MagicMock, patch - -from clayde.claude import InvocationResult, InvocationTimeoutError, UsageLimitError -from clayde.prompts import collect_comments_after -from clayde.tasks.implement import ( - _assign_reviewer_and_finish, - _checkout_wip_branch, - _post_result, - _prepare_implementation_context, - run, -) - - -def _make_result(output: str, cost_eur: float = 0.50) -> InvocationResult: - """Helper to create an InvocationResult for testing.""" - return InvocationResult(output=output, cost_eur=cost_eur, input_tokens=100, output_tokens=50) - - -class TestCollectDiscussion: - def test_collects_comments_after_plan(self): - plan_comment = MagicMock() - plan_comment.id = 100 - - c1 = MagicMock() - c1.id = 99 - c1.user.login = "alice" - c1.body = "before plan" - - c2 = MagicMock() - c2.id = 100 # the plan comment - - c3 = MagicMock() - c3.id = 101 - c3.user.login = "bob" - c3.body = "after plan" - - result = collect_comments_after([c1, c2, c3], 100) - assert "@bob" in result - assert "after plan" in result - assert "before plan" not in result - - def test_no_discussion(self): - plan = MagicMock() - plan.id = 100 - result = collect_comments_after([plan], 100) - assert result == "(none)" - - def test_empty_comments(self): - assert collect_comments_after([], 100) == "(none)" - - -class TestPostResult: - def test_posts_with_pr_url(self): - g = MagicMock() - _post_result(g, "o", "r", 1, "https://github.com/o/r/pull/5") - body = g.get_repo.return_value.get_issue.return_value.create_comment.call_args[0][0] - assert "https://github.com/o/r/pull/5" in body - # No cost line when cost_eur is None - assert "💸" not in body - - def test_posts_pr_url(self): - g = MagicMock() - _post_result(g, "o", "r", 1, "https://github.com/o/r/pull/7") - body = g.get_repo.return_value.get_issue.return_value.create_comment.call_args[0][0] - assert "https://github.com/o/r/pull/7" in body - assert "complete" in body.lower() - - def test_posts_with_cost(self): - g = MagicMock() - _post_result(g, "o", "r", 1, "https://github.com/o/r/pull/5", cost_eur=3.50) - body = g.get_repo.return_value.get_issue.return_value.create_comment.call_args[0][0] - assert "💸 This task cost 3.50€" in body - - -class TestAssignReviewerAndFinish: - def test_assigns_reviewer_and_sets_pr_open(self): - g = MagicMock() - span = MagicMock() - with patch("clayde.tasks.implement.get_issue_author", return_value="alice"), \ - patch("clayde.tasks.implement.parse_pr_url", return_value=("o", "r", 5)), \ - patch("clayde.tasks.implement.add_pr_reviewer") as mock_reviewer, \ - patch("clayde.tasks.implement.post_comment") as mock_post, \ - patch("clayde.tasks.implement.update_issue_state") as mock_update: - _assign_reviewer_and_finish(g, "o", "r", 1, "url", "https://github.com/o/r/pull/5", span) - - mock_reviewer.assert_called_once_with(g, "o", "r", 5, "alice") - mock_update.assert_called_once() - update_data = mock_update.call_args[0][1] - assert update_data["status"] == "pr_open" - assert update_data["pr_url"] == "https://github.com/o/r/pull/5" - assert update_data["last_seen_review_id"] == 0 - - def test_passes_cost_to_post_result(self): - g = MagicMock() - span = MagicMock() - with patch("clayde.tasks.implement.get_issue_author", return_value="alice"), \ - patch("clayde.tasks.implement.parse_pr_url", return_value=("o", "r", 5)), \ - patch("clayde.tasks.implement.add_pr_reviewer"), \ - patch("clayde.tasks.implement.post_comment") as mock_post, \ - patch("clayde.tasks.implement.update_issue_state"): - _assign_reviewer_and_finish(g, "o", "r", 1, "url", "https://github.com/o/r/pull/5", span, - cost_eur=4.20) - - posted_body = mock_post.call_args[0][4] - assert "💸 This task cost 4.20€" in posted_body - - def test_handles_reviewer_failure_gracefully(self): - g = MagicMock() - span = MagicMock() - with patch("clayde.tasks.implement.get_issue_author", side_effect=Exception("fail")), \ - patch("clayde.tasks.implement.post_comment"), \ - patch("clayde.tasks.implement.update_issue_state") as mock_update: - # Should not raise - _assign_reviewer_and_finish(g, "o", "r", 1, "url", "https://github.com/o/r/pull/5", span) - - # Status should still be set to pr_open - mock_update.assert_called_once() - assert mock_update.call_args[0][1]["status"] == "pr_open" - - -class TestRun: - def test_full_success_creates_pr_and_assigns_reviewer(self, tmp_path): - with patch("clayde.tasks.implement.get_github_client") as mock_gc, \ - patch("clayde.tasks.implement.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.implement.get_issue_state", return_value={"plan_comment_id": 100}), \ - patch("clayde.tasks.implement.update_issue_state") as mock_update, \ - patch("clayde.tasks.implement.fetch_issue") as mock_fi, \ - patch("clayde.tasks.implement.get_default_branch", return_value="main"), \ - patch("clayde.tasks.implement.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.implement.fetch_comment") as mock_fc, \ - patch("clayde.tasks.implement.fetch_issue_comments", return_value=[]), \ - patch("clayde.tasks.implement.filter_comments", return_value=[]), \ - patch("clayde.tasks.implement._build_prompt", return_value="prompt"), \ - patch("clayde.tasks.implement.invoke_claude", return_value=_make_result("IMPLEMENTATION_COMPLETE", cost_eur=5.00)), \ - patch("clayde.tasks.implement.find_open_pr", return_value=None), \ - patch("clayde.tasks.implement._ensure_branch_pushed", return_value=True), \ - patch("clayde.tasks.implement.create_pull_request", return_value="https://github.com/o/r/pull/5") as mock_cpr, \ - patch("clayde.tasks.implement._assign_reviewer_and_finish") as mock_finish, \ - patch("clayde.tasks.implement.pop_accumulated_cost", return_value=0.0), \ - patch("clayde.tasks.implement.DATA_DIR", tmp_path): - mock_fc.return_value.body = "plan text" - mock_fi.return_value.title = "Test issue" - run("https://github.com/o/r/issues/1") - - mock_cpr.assert_called_once() - # PR body should include cost - pr_body = mock_cpr.call_args.kwargs.get("body") or mock_cpr.call_args[1].get("body", "") - assert "💸 This task cost 5.00€" in pr_body - mock_finish.assert_called_once() - # Cost is passed to _assign_reviewer_and_finish - assert mock_finish.call_args.kwargs.get("cost_eur") == 5.00 or mock_finish.call_args[1].get("cost_eur") == 5.00 - - def test_existing_pr_reused(self, tmp_path): - with patch("clayde.tasks.implement.get_github_client") as mock_gc, \ - patch("clayde.tasks.implement.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.implement.get_issue_state", return_value={"plan_comment_id": 100}), \ - patch("clayde.tasks.implement.update_issue_state") as mock_update, \ - patch("clayde.tasks.implement.fetch_issue") as mock_fi, \ - patch("clayde.tasks.implement.get_default_branch", return_value="main"), \ - patch("clayde.tasks.implement.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.implement.fetch_comment") as mock_fc, \ - patch("clayde.tasks.implement.fetch_issue_comments", return_value=[]), \ - patch("clayde.tasks.implement.filter_comments", return_value=[]), \ - patch("clayde.tasks.implement._build_prompt", return_value="prompt"), \ - patch("clayde.tasks.implement.invoke_claude", return_value=_make_result("IMPLEMENTATION_COMPLETE")), \ - patch("clayde.tasks.implement.find_open_pr", return_value="https://github.com/o/r/pull/5"), \ - patch("clayde.tasks.implement.create_pull_request") as mock_cpr, \ - patch("clayde.tasks.implement._assign_reviewer_and_finish") as mock_finish, \ - patch("clayde.tasks.implement.pop_accumulated_cost", return_value=0.0), \ - patch("clayde.tasks.implement.DATA_DIR", tmp_path): - mock_fc.return_value.body = "plan text" - run("https://github.com/o/r/issues/1") - - mock_cpr.assert_not_called() - mock_finish.assert_called_once() - - def test_usage_limit_sets_interrupted_and_accumulates_cost(self, tmp_path): - with patch("clayde.tasks.implement.get_github_client"), \ - patch("clayde.tasks.implement.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.implement.get_issue_state", return_value={"plan_comment_id": 100}), \ - patch("clayde.tasks.implement.update_issue_state") as mock_update, \ - patch("clayde.tasks.implement.fetch_issue"), \ - patch("clayde.tasks.implement.get_default_branch", return_value="main"), \ - patch("clayde.tasks.implement.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.implement.fetch_comment") as mock_fc, \ - patch("clayde.tasks.implement.fetch_issue_comments", return_value=[]), \ - patch("clayde.tasks.implement.filter_comments", return_value=[]), \ - patch("clayde.tasks.implement._build_prompt", return_value="prompt"), \ - patch("clayde.tasks.implement.invoke_claude", side_effect=UsageLimitError("limit", cost_eur=2.00)), \ - patch("clayde.tasks.implement.accumulate_cost") as mock_accum, \ - patch("clayde.tasks.implement.DATA_DIR", tmp_path): - mock_fc.return_value.body = "plan text" - run("url") - - last_call = mock_update.call_args_list[-1] - assert last_call[0][1]["status"] == "interrupted" - assert last_call[0][1]["interrupted_phase"] == "implementing" - mock_accum.assert_called_once_with("url", 2.00) - - def test_timeout_sets_interrupted_and_accumulates_cost(self, tmp_path): - with patch("clayde.tasks.implement.get_github_client"), \ - patch("clayde.tasks.implement.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.implement.get_issue_state", return_value={"plan_comment_id": 100}), \ - patch("clayde.tasks.implement.update_issue_state") as mock_update, \ - patch("clayde.tasks.implement.fetch_issue"), \ - patch("clayde.tasks.implement.get_default_branch", return_value="main"), \ - patch("clayde.tasks.implement.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.implement.fetch_comment") as mock_fc, \ - patch("clayde.tasks.implement.fetch_issue_comments", return_value=[]), \ - patch("clayde.tasks.implement.filter_comments", return_value=[]), \ - patch("clayde.tasks.implement._build_prompt", return_value="prompt"), \ - patch("clayde.tasks.implement.invoke_claude", side_effect=InvocationTimeoutError("timeout", cost_eur=1.50)), \ - patch("clayde.tasks.implement.accumulate_cost") as mock_accum, \ - patch("clayde.tasks.implement.DATA_DIR", tmp_path): - mock_fc.return_value.body = "plan text" - run("url") - - last_call = mock_update.call_args_list[-1] - assert last_call[0][1]["status"] == "interrupted" - assert last_call[0][1]["interrupted_phase"] == "implementing" - mock_accum.assert_called_once_with("url", 1.50) - - def test_resumes_interrupted_with_existing_pr(self): - state = {"plan_comment_id": 100, "status": "interrupted"} - with patch("clayde.tasks.implement.get_github_client") as mock_gc, \ - patch("clayde.tasks.implement.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.implement.get_issue_state", return_value=state), \ - patch("clayde.tasks.implement.find_open_pr", return_value="https://github.com/o/r/pull/5"), \ - patch("clayde.tasks.implement._assign_reviewer_and_finish") as mock_finish, \ - patch("clayde.tasks.implement.pop_accumulated_cost", return_value=3.00), \ - patch("clayde.tasks.implement.invoke_claude") as mock_claude: - run("url") - mock_claude.assert_not_called() - - mock_finish.assert_called_once() - # Accumulated cost should be passed - finish_kwargs = mock_finish.call_args - assert finish_kwargs.kwargs.get("cost_eur") == 3.00 - - def test_resumes_interrupted_with_existing_pr_no_accumulated_cost(self): - state = {"plan_comment_id": 100, "status": "interrupted"} - with patch("clayde.tasks.implement.get_github_client") as mock_gc, \ - patch("clayde.tasks.implement.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.implement.get_issue_state", return_value=state), \ - patch("clayde.tasks.implement.find_open_pr", return_value="https://github.com/o/r/pull/5"), \ - patch("clayde.tasks.implement._assign_reviewer_and_finish") as mock_finish, \ - patch("clayde.tasks.implement.pop_accumulated_cost", return_value=0.0), \ - patch("clayde.tasks.implement.invoke_claude") as mock_claude: - run("url") - mock_claude.assert_not_called() - - mock_finish.assert_called_once() - # No cost to report when accumulated is 0 - finish_kwargs = mock_finish.call_args - assert finish_kwargs.kwargs.get("cost_eur") is None - - def test_pr_creation_failure_sets_interrupted(self, tmp_path): - with patch("clayde.tasks.implement.get_github_client") as mock_gc, \ - patch("clayde.tasks.implement.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.implement.get_issue_state", return_value={"plan_comment_id": 100}), \ - patch("clayde.tasks.implement.update_issue_state") as mock_update, \ - patch("clayde.tasks.implement.fetch_issue") as mock_fi, \ - patch("clayde.tasks.implement.get_default_branch", return_value="main"), \ - patch("clayde.tasks.implement.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.implement.fetch_comment") as mock_fc, \ - patch("clayde.tasks.implement.fetch_issue_comments", return_value=[]), \ - patch("clayde.tasks.implement.filter_comments", return_value=[]), \ - patch("clayde.tasks.implement._build_prompt", return_value="prompt"), \ - patch("clayde.tasks.implement.invoke_claude", return_value=_make_result("IMPLEMENTATION_COMPLETE")), \ - patch("clayde.tasks.implement.find_open_pr", return_value=None), \ - patch("clayde.tasks.implement._ensure_branch_pushed", return_value=True), \ - patch("clayde.tasks.implement.create_pull_request", side_effect=Exception("API error")), \ - patch("clayde.tasks.implement.post_comment"), \ - patch("clayde.tasks.implement.pop_accumulated_cost", return_value=0.0), \ - patch("clayde.tasks.implement.DATA_DIR", tmp_path): - mock_fc.return_value.body = "plan text" - mock_fi.return_value.title = "Test issue" - run("https://github.com/o/r/issues/1") - - last_call = mock_update.call_args_list[-1] - assert last_call[0][1]["status"] == "interrupted" - assert last_call[0][1]["interrupted_phase"] == "implementing" - assert last_call[0][1]["retry_count"] == 1 - - def test_no_pr_fails_after_max_retries(self, tmp_path): - with patch("clayde.tasks.implement.get_github_client") as mock_gc, \ - patch("clayde.tasks.implement.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.implement.get_issue_state", return_value={"plan_comment_id": 100, "retry_count": 2}), \ - patch("clayde.tasks.implement.update_issue_state") as mock_update, \ - patch("clayde.tasks.implement.fetch_issue") as mock_fi, \ - patch("clayde.tasks.implement.get_default_branch", return_value="main"), \ - patch("clayde.tasks.implement.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.implement.fetch_comment") as mock_fc, \ - patch("clayde.tasks.implement.fetch_issue_comments", return_value=[]), \ - patch("clayde.tasks.implement.filter_comments", return_value=[]), \ - patch("clayde.tasks.implement._build_prompt", return_value="prompt"), \ - patch("clayde.tasks.implement.invoke_claude", return_value=_make_result("IMPLEMENTATION_COMPLETE")), \ - patch("clayde.tasks.implement.find_open_pr", return_value=None), \ - patch("clayde.tasks.implement._ensure_branch_pushed", return_value=True), \ - patch("clayde.tasks.implement.create_pull_request", side_effect=Exception("API error")), \ - patch("clayde.tasks.implement.post_comment"), \ - patch("clayde.tasks.implement.pop_accumulated_cost", return_value=0.0), \ - patch("clayde.tasks.implement.DATA_DIR", tmp_path): - mock_fc.return_value.body = "plan text" - mock_fi.return_value.title = "Test issue" - run("https://github.com/o/r/issues/1") - - last_call = mock_update.call_args_list[-1] - assert last_call[0][1]["status"] == "failed" - assert last_call[0][1]["retry_count"] == 3 - - def test_build_prompt_uses_real_template(self): - """Test that _build_prompt renders with the real Jinja2 template.""" - from clayde.tasks.implement import _build_prompt - - issue = MagicMock() - issue.title = "Test issue" - issue.body = "Fix this bug" - - prompt = _build_prompt(issue, "plan text", "discussion", "owner", "repo", 42, "/tmp/repo", "clayde/issue-42-test-branch") - assert "Test issue" in prompt - assert "Fix this bug" in prompt - assert "plan text" in prompt - assert "discussion" in prompt - assert "/tmp/repo" in prompt - assert "42" in prompt - assert "clayde/issue-42-test-branch" in prompt - - def test_conversation_path_passed_to_invoke_claude(self): - with patch("clayde.tasks.implement.get_github_client") as mock_gc, \ - patch("clayde.tasks.implement.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.implement.get_issue_state", return_value={"plan_comment_id": 100}), \ - patch("clayde.tasks.implement.update_issue_state"), \ - patch("clayde.tasks.implement.fetch_issue") as mock_fi, \ - patch("clayde.tasks.implement.get_default_branch", return_value="main"), \ - patch("clayde.tasks.implement.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.implement.fetch_comment") as mock_fc, \ - patch("clayde.tasks.implement.fetch_issue_comments", return_value=[]), \ - patch("clayde.tasks.implement.filter_comments", return_value=[]), \ - patch("clayde.tasks.implement._build_prompt", return_value="prompt"), \ - patch("clayde.tasks.implement.invoke_claude", return_value=_make_result("done")) as mock_claude, \ - patch("clayde.tasks.implement.find_open_pr", return_value="https://github.com/o/r/pull/5"), \ - patch("clayde.tasks.implement._assign_reviewer_and_finish"), \ - patch("clayde.tasks.implement.pop_accumulated_cost", return_value=0.0), \ - patch("clayde.tasks.implement.DATA_DIR", Path("/tmp/test-data")): - mock_fc.return_value.body = "plan text" - mock_fi.return_value.title = "Test" - run("https://github.com/o/r/issues/1") - - call_kwargs = mock_claude.call_args - assert call_kwargs.kwargs["branch_name"] is not None - assert call_kwargs.kwargs["conversation_path"] is not None - assert "o__r__issue-1" in str(call_kwargs.kwargs["conversation_path"]) - - def test_resumed_issue_checks_out_wip_branch(self): - state = { - "plan_comment_id": 100, - "status": "interrupted", - "branch_name": "clayde/issue-1-fix", - } - with patch("clayde.tasks.implement.get_github_client") as mock_gc, \ - patch("clayde.tasks.implement.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.implement.get_issue_state", return_value=state), \ - patch("clayde.tasks.implement.find_open_pr", return_value=None), \ - patch("clayde.tasks.implement.update_issue_state"), \ - patch("clayde.tasks.implement.fetch_issue") as mock_fi, \ - patch("clayde.tasks.implement.get_default_branch", return_value="main"), \ - patch("clayde.tasks.implement.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.implement.fetch_comment") as mock_fc, \ - patch("clayde.tasks.implement.fetch_issue_comments", return_value=[]), \ - patch("clayde.tasks.implement.filter_comments", return_value=[]), \ - patch("clayde.tasks.implement._build_prompt", return_value="prompt"), \ - patch("clayde.tasks.implement.invoke_claude", return_value=_make_result("done")), \ - patch("clayde.tasks.implement._ensure_branch_pushed", return_value=True), \ - patch("clayde.tasks.implement.create_pull_request", return_value="https://github.com/o/r/pull/5"), \ - patch("clayde.tasks.implement._assign_reviewer_and_finish"), \ - patch("clayde.tasks.implement._checkout_wip_branch") as mock_checkout, \ - patch("clayde.tasks.implement.pop_accumulated_cost", return_value=0.0), \ - patch("clayde.tasks.implement.DATA_DIR", Path("/tmp/test-data")): - mock_fc.return_value.body = "plan text" - mock_fi.return_value.title = "Test" - run("https://github.com/o/r/issues/1") - - mock_checkout.assert_called_once_with("/tmp/repo", "clayde/issue-1-fix") - - -class TestCheckoutWipBranch: - def test_checks_out_local_branch(self): - calls = [] - - def fake_run(cmd, **kwargs): - calls.append(cmd) - result = MagicMock() - result.returncode = 0 - if cmd == ["git", "branch", "--list", "clayde/issue-1"]: - result.stdout = " clayde/issue-1\n" - else: - result.stdout = "" - return result - - with patch("clayde.tasks.implement.subprocess.run", side_effect=fake_run): - _checkout_wip_branch("/repo", "clayde/issue-1") - - cmd_strs = [" ".join(c) for c in calls] - assert any("checkout clayde/issue-1" in s for s in cmd_strs) - - def test_checks_out_remote_branch(self): - calls = [] - - def fake_run(cmd, **kwargs): - calls.append(cmd) - result = MagicMock() - result.returncode = 0 - if cmd == ["git", "branch", "--list", "clayde/issue-1"]: - result.stdout = "" # not local - elif "ls-remote" in cmd: - result.stdout = "abc123\trefs/heads/clayde/issue-1\n" - else: - result.stdout = "" - return result - - with patch("clayde.tasks.implement.subprocess.run", side_effect=fake_run): - _checkout_wip_branch("/repo", "clayde/issue-1") - - cmd_strs = [" ".join(c) for c in calls] - assert any("checkout -b clayde/issue-1 origin/clayde/issue-1" in s for s in cmd_strs) - - def test_no_branch_found_does_nothing(self): - def fake_run(cmd, **kwargs): - result = MagicMock() - result.returncode = 0 - result.stdout = "" - return result - - with patch("clayde.tasks.implement.subprocess.run", side_effect=fake_run): - _checkout_wip_branch("/repo", "clayde/issue-1") # Should not raise - - -class TestDeleteConversationFile: - def test_fresh_run_deletes_stale_conversation_file(self, tmp_path): - """When starting a fresh (non-resumed) implementation, stale conversation files are deleted.""" - conv_dir = tmp_path / "conversations" - conv_dir.mkdir() - conv_file = conv_dir / "o__r__issue-1.json" - conv_file.write_text('{"session_id": "stale"}') - - with patch("clayde.tasks.implement.get_github_client"), \ - patch("clayde.tasks.implement.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.implement.get_issue_state", return_value={"plan_comment_id": 100}), \ - patch("clayde.tasks.implement.update_issue_state"), \ - patch("clayde.tasks.implement.fetch_issue") as mock_fi, \ - patch("clayde.tasks.implement.get_default_branch", return_value="main"), \ - patch("clayde.tasks.implement.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.implement.fetch_comment") as mock_fc, \ - patch("clayde.tasks.implement.fetch_issue_comments", return_value=[]), \ - patch("clayde.tasks.implement.filter_comments", return_value=[]), \ - patch("clayde.tasks.implement._build_prompt", return_value="prompt"), \ - patch("clayde.tasks.implement.invoke_claude", return_value=_make_result("done")), \ - patch("clayde.tasks.implement.find_open_pr", return_value="https://github.com/o/r/pull/5"), \ - patch("clayde.tasks.implement._assign_reviewer_and_finish"), \ - patch("clayde.tasks.implement.pop_accumulated_cost", return_value=0.0), \ - patch("clayde.tasks.implement.DATA_DIR", tmp_path): - mock_fc.return_value.body = "plan text" - mock_fi.return_value.title = "Test" - run("https://github.com/o/r/issues/1") - - assert not conv_file.exists() - - def test_failed_after_retries_deletes_conversation_file(self, tmp_path): - """When giving up after max retries, the conversation file is cleaned up.""" - conv_dir = tmp_path / "conversations" - conv_dir.mkdir() - conv_file = conv_dir / "o__r__issue-1.json" - conv_file.write_text('{"session_id": "stale"}') - - with patch("clayde.tasks.implement.get_github_client"), \ - patch("clayde.tasks.implement.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.implement.get_issue_state", return_value={"plan_comment_id": 100, "retry_count": 2}), \ - patch("clayde.tasks.implement.update_issue_state"), \ - patch("clayde.tasks.implement.fetch_issue") as mock_fi, \ - patch("clayde.tasks.implement.get_default_branch", return_value="main"), \ - patch("clayde.tasks.implement.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.implement.fetch_comment") as mock_fc, \ - patch("clayde.tasks.implement.fetch_issue_comments", return_value=[]), \ - patch("clayde.tasks.implement.filter_comments", return_value=[]), \ - patch("clayde.tasks.implement._build_prompt", return_value="prompt"), \ - patch("clayde.tasks.implement.invoke_claude", return_value=_make_result("done")), \ - patch("clayde.tasks.implement.find_open_pr", return_value=None), \ - patch("clayde.tasks.implement._ensure_branch_pushed", return_value=True), \ - patch("clayde.tasks.implement.create_pull_request", side_effect=Exception("fail")), \ - patch("clayde.tasks.implement.post_comment"), \ - patch("clayde.tasks.implement.pop_accumulated_cost", return_value=0.0), \ - patch("clayde.tasks.implement.DATA_DIR", tmp_path): - mock_fc.return_value.body = "plan text" - mock_fi.return_value.title = "Test" - run("https://github.com/o/r/issues/1") - - assert not conv_file.exists() - - -class TestPrepareImplementationContext: - """Tests for _prepare_implementation_context.""" - - def test_falls_back_to_preliminary_comment_id(self, tmp_path): - """Small issues have no plan_comment_id; should use preliminary_comment_id.""" - g = MagicMock() - issue_state = {"preliminary_comment_id": 42, "branch_name": "clayde/issue-1"} - - with patch("clayde.tasks.implement.fetch_issue") as mock_fi, \ - patch("clayde.tasks.implement.get_default_branch", return_value="main"), \ - patch("clayde.tasks.implement.ensure_repo", return_value=tmp_path), \ - patch("clayde.tasks.implement.fetch_comment") as mock_fc, \ - patch("clayde.tasks.implement.update_issue_state"), \ - patch("clayde.tasks.implement.fetch_issue_comments", return_value=[]), \ - patch("clayde.tasks.implement.filter_comments", return_value=[]), \ - patch("clayde.tasks.implement.DATA_DIR", tmp_path): - mock_fi.return_value.title = "Test" - mock_fc.return_value.body = "preliminary plan text" - - _prepare_implementation_context( - g, "o", "r", 1, "https://github.com/o/r/issues/1", issue_state, False, - ) - - mock_fc.assert_called_once_with(g, "o", "r", 1, 42) diff --git a/tests/test_tasks_plan.py b/tests/test_tasks_plan.py deleted file mode 100644 index 4ec9bfb..0000000 --- a/tests/test_tasks_plan.py +++ /dev/null @@ -1,527 +0,0 @@ -"""Tests for clayde.tasks.plan — two-phase planning.""" - -from unittest.mock import MagicMock, patch - -from clayde.claude import InvocationResult, UsageLimitError -from clayde.prompts import collect_comments_after -from clayde.tasks.plan import ( - _build_preliminary_prompt, - _build_thorough_prompt, - _build_update_prompt, - _post_preliminary_comment, - _post_thorough_plan_comment, - run_preliminary, - run_thorough, - run_update, -) - - -def _make_result(output: str, cost_eur: float = 0.50) -> InvocationResult: - """Helper to create an InvocationResult for testing.""" - return InvocationResult(output=output, cost_eur=cost_eur, input_tokens=100, output_tokens=50) - - -def _mock_settings(users=None): - s = MagicMock() - s.whitelisted_users_list = users or ["alice"] - s.github_username = "ClaydeCode" - return s - - -class TestBuildPreliminaryPrompt: - def test_renders_with_issue_data(self): - g = MagicMock() - issue = MagicMock() - issue.title = "Fix bug" - issue.body = "There is a bug" - issue.labels = [] - issue.user.login = "alice" - - comment = MagicMock() - comment.user.login = "alice" - comment.body = "I can confirm this" - comment.get_reactions.return_value = [] - g.get_repo.return_value.get_issue.return_value.get_comments.return_value = [comment] - - settings = _mock_settings() - with patch("clayde.safety.get_settings", return_value=settings): - prompt = _build_preliminary_prompt(g, issue, "owner", "repo", 42, "/tmp/repo") - assert "Fix bug" in prompt - assert "There is a bug" in prompt - assert "42" in prompt - assert "/tmp/repo" in prompt - # Should be a preliminary plan prompt, not thorough - assert "preliminary" in prompt.lower() or "short" in prompt.lower() - - def test_filters_invisible_comments(self): - g = MagicMock() - issue = MagicMock() - issue.title = "Title" - issue.body = "body" - issue.labels = [] - issue.user.login = "alice" - - visible = MagicMock() - visible.user.login = "alice" - visible.body = "visible comment" - visible.get_reactions.return_value = [] - - invisible = MagicMock() - invisible.user.login = "bob" - invisible.body = "invisible comment" - invisible.get_reactions.return_value = [] - - g.get_repo.return_value.get_issue.return_value.get_comments.return_value = [visible, invisible] - - settings = _mock_settings() - with patch("clayde.safety.get_settings", return_value=settings): - prompt = _build_preliminary_prompt(g, issue, "owner", "repo", 1, "/path") - assert "visible comment" in prompt - assert "invisible comment" not in prompt - - def test_filters_issue_body_when_not_visible(self): - g = MagicMock() - issue = MagicMock() - issue.title = "Title" - issue.body = "secret body" - issue.labels = [] - issue.user.login = "bob" # not whitelisted - issue.get_reactions.return_value = [] - - g.get_repo.return_value.get_issue.return_value.get_comments.return_value = [] - - settings = _mock_settings() - with patch("clayde.safety.get_settings", return_value=settings): - prompt = _build_preliminary_prompt(g, issue, "owner", "repo", 1, "/path") - assert "secret body" not in prompt - assert "(filtered)" in prompt - - -class TestBuildThoroughPrompt: - def test_renders_with_preliminary_plan(self): - g = MagicMock() - issue = MagicMock() - issue.title = "Fix bug" - issue.body = "body" - issue.labels = [] - issue.user.login = "alice" - - settings = _mock_settings() - with patch("clayde.safety.get_settings", return_value=settings): - prompt = _build_thorough_prompt( - g, issue, "owner", "repo", 1, "/path", - "my preliminary plan", "discussion text", - ) - assert "my preliminary plan" in prompt - assert "discussion text" in prompt - assert "implementation plan" in prompt.lower() - - -class TestBuildUpdatePrompt: - def test_renders_with_new_comments(self): - prompt = _build_update_prompt( - 1, "Title", "o", "r", "body", - "current plan text", "new comment text", "/path", - phase="preliminary", - ) - assert "current plan text" in prompt - assert "new comment text" in prompt - assert "summary" in prompt.lower() - assert "updated_plan" in prompt - - def test_preliminary_phase_warns_against_escalation(self): - prompt = _build_update_prompt( - 1, "Title", "o", "r", "body", - "current plan text", "new comment text", "/path", - phase="preliminary", - ) - assert "PRELIMINARY" in prompt - assert "Do NOT escalate" in prompt - - def test_thorough_phase_maintains_detail(self): - prompt = _build_update_prompt( - 1, "Title", "o", "r", "body", - "current plan text", "new comment text", "/path", - phase="thorough", - ) - assert "thorough implementation plan" in prompt - - -class TestPostPreliminaryComment: - def test_posts_formatted_comment_large(self): - g = MagicMock() - mock_comment = MagicMock() - mock_comment.id = 555 - g.get_repo.return_value.get_issue.return_value.create_comment.return_value = mock_comment - - result = _post_preliminary_comment(g, "owner", "repo", 1, "My preliminary plan", size="large") - assert result == 555 - posted_body = g.get_repo.return_value.get_issue.return_value.create_comment.call_args[0][0] - assert "## Preliminary Plan" in posted_body - assert "My preliminary plan" in posted_body - assert "\U0001f44d" in posted_body - assert "large" in posted_body - assert "thorough implementation plan" in posted_body - assert "💸" not in posted_body # no cost line when cost is 0 - - def test_posts_formatted_comment_small(self): - g = MagicMock() - mock_comment = MagicMock() - mock_comment.id = 556 - g.get_repo.return_value.get_issue.return_value.create_comment.return_value = mock_comment - - result = _post_preliminary_comment(g, "owner", "repo", 1, "My small plan", size="small") - assert result == 556 - posted_body = g.get_repo.return_value.get_issue.return_value.create_comment.call_args[0][0] - assert "small" in posted_body - assert "directly to implementation" in posted_body - - def test_posts_with_cost(self): - g = MagicMock() - mock_comment = MagicMock() - mock_comment.id = 555 - g.get_repo.return_value.get_issue.return_value.create_comment.return_value = mock_comment - - result = _post_preliminary_comment(g, "owner", "repo", 1, "plan", cost_eur=1.23) - posted_body = g.get_repo.return_value.get_issue.return_value.create_comment.call_args[0][0] - assert "💸 This task cost 1.23€" in posted_body - - -class TestPostThoroughPlanComment: - def test_posts_formatted_comment(self): - g = MagicMock() - mock_comment = MagicMock() - mock_comment.id = 666 - g.get_repo.return_value.get_issue.return_value.create_comment.return_value = mock_comment - - result = _post_thorough_plan_comment(g, "owner", "repo", 1, "My thorough plan") - assert result == 666 - posted_body = g.get_repo.return_value.get_issue.return_value.create_comment.call_args[0][0] - assert "## Implementation Plan" in posted_body - assert "My thorough plan" in posted_body - assert "\U0001f44d" in posted_body - assert "💸" not in posted_body # no cost line when cost is 0 - - def test_posts_with_cost(self): - g = MagicMock() - mock_comment = MagicMock() - mock_comment.id = 666 - g.get_repo.return_value.get_issue.return_value.create_comment.return_value = mock_comment - - result = _post_thorough_plan_comment(g, "owner", "repo", 1, "plan", cost_eur=3.45) - posted_body = g.get_repo.return_value.get_issue.return_value.create_comment.call_args[0][0] - assert "💸 This task cost 3.45€" in posted_body - - -class TestCollectDiscussionAfter: - def test_collects_comments_after_id(self): - c1 = MagicMock() - c1.id = 100 - c1.user.login = "plan" - c1.body = "plan text" - - c2 = MagicMock() - c2.id = 101 - c2.user.login = "alice" - c2.body = "discussion" - - result = collect_comments_after([c1, c2], 100) - assert "discussion" in result - assert "plan text" not in result - - def test_none_when_no_comments_after(self): - c1 = MagicMock() - c1.id = 100 - result = collect_comments_after([c1], 100) - assert result == "(none)" - - -class TestRunPreliminary: - def test_full_success(self): - mock_comment = MagicMock() - mock_comment.id = 500 - - mock_issue = MagicMock() - mock_issue.title = "Test issue" - - with patch("clayde.tasks.plan.get_github_client") as mock_gc, \ - patch("clayde.tasks.plan.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.plan.update_issue_state") as mock_update, \ - patch("clayde.tasks.plan.fetch_issue", return_value=mock_issue) as mock_fi, \ - patch("clayde.tasks.plan.get_default_branch", return_value="main"), \ - patch("clayde.tasks.plan.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.plan._build_preliminary_prompt", return_value="prompt"), \ - patch("clayde.tasks.plan.invoke_claude", return_value=_make_result( - '{"plan": "' + "x" * 150 + '", "size": "large", "branch_name": "clayde/issue-1-fix"}', cost_eur=1.00)), \ - patch("clayde.tasks.plan._post_preliminary_comment", return_value=999) as mock_post, \ - patch("clayde.tasks.plan.fetch_issue_comments", return_value=[mock_comment]), \ - patch("clayde.tasks.plan.pop_accumulated_cost", return_value=0.0): - run_preliminary("https://github.com/o/r/issues/1") - - calls = mock_update.call_args_list - assert calls[0][0] == ("https://github.com/o/r/issues/1", - {"status": "preliminary_planning", "owner": "o", "repo": "r", "number": 1, - "issue_title": "Test issue"}) - last = calls[-1][0][1] - assert last["status"] == "awaiting_preliminary_approval" - assert last["preliminary_comment_id"] == 999 - assert last["last_seen_comment_id"] == 500 - assert last["size"] == "large" - assert last["branch_name"] == "clayde/issue-1-fix" - # Cost is passed to the comment helper - mock_post.assert_called_once() - assert mock_post.call_args[0][4] == "x" * 150 - assert mock_post.call_args[0][5] == 1.00 - - def test_empty_plan_sets_failed(self): - with patch("clayde.tasks.plan.get_github_client"), \ - patch("clayde.tasks.plan.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.plan.update_issue_state") as mock_update, \ - patch("clayde.tasks.plan.fetch_issue"), \ - patch("clayde.tasks.plan.get_default_branch", return_value="main"), \ - patch("clayde.tasks.plan.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.plan._build_preliminary_prompt", return_value="prompt"), \ - patch("clayde.tasks.plan.invoke_claude", return_value=_make_result(" ")), \ - patch("clayde.tasks.plan.pop_accumulated_cost", return_value=0.0): - run_preliminary("url") - - last_call = mock_update.call_args_list[-1] - # Empty output → failed - assert last_call[0][1]["status"] == "failed" - - def test_invalid_json_sets_failed(self): - with patch("clayde.tasks.plan.get_github_client"), \ - patch("clayde.tasks.plan.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.plan.update_issue_state") as mock_update, \ - patch("clayde.tasks.plan.fetch_issue"), \ - patch("clayde.tasks.plan.get_default_branch", return_value="main"), \ - patch("clayde.tasks.plan.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.plan._build_preliminary_prompt", return_value="prompt"), \ - patch("clayde.tasks.plan.invoke_claude", return_value=_make_result("not valid json at all")), \ - patch("clayde.tasks.plan.pop_accumulated_cost", return_value=0.0): - run_preliminary("url") - - last_call = mock_update.call_args_list[-1] - assert last_call[0][1]["status"] == "failed" - - def test_usage_limit_sets_interrupted_and_accumulates_cost(self): - with patch("clayde.tasks.plan.get_github_client"), \ - patch("clayde.tasks.plan.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.plan.update_issue_state") as mock_update, \ - patch("clayde.tasks.plan.fetch_issue"), \ - patch("clayde.tasks.plan.get_default_branch", return_value="main"), \ - patch("clayde.tasks.plan.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.plan._build_preliminary_prompt", return_value="prompt"), \ - patch("clayde.tasks.plan.invoke_claude", side_effect=UsageLimitError("limit", cost_eur=0.75)), \ - patch("clayde.tasks.plan.accumulate_cost") as mock_accum: - run_preliminary("url") - - last_call = mock_update.call_args_list[-1] - assert last_call[0][1]["status"] == "interrupted" - assert last_call[0][1]["interrupted_phase"] == "preliminary_planning" - mock_accum.assert_called_once_with("url", 0.75) - - def test_accumulated_cost_included_on_success(self): - """Cost from prior interrupted runs is included in the total.""" - mock_comment = MagicMock() - mock_comment.id = 500 - - with patch("clayde.tasks.plan.get_github_client"), \ - patch("clayde.tasks.plan.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.plan.update_issue_state"), \ - patch("clayde.tasks.plan.fetch_issue"), \ - patch("clayde.tasks.plan.get_default_branch", return_value="main"), \ - patch("clayde.tasks.plan.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.plan._build_preliminary_prompt", return_value="prompt"), \ - patch("clayde.tasks.plan.invoke_claude", return_value=_make_result( - '{"plan": "' + "x" * 150 + '", "size": "small", "branch_name": "clayde/issue-1-fix"}', cost_eur=1.00)), \ - patch("clayde.tasks.plan._post_preliminary_comment", return_value=999) as mock_post, \ - patch("clayde.tasks.plan.fetch_issue_comments", return_value=[mock_comment]), \ - patch("clayde.tasks.plan.pop_accumulated_cost", return_value=2.00): - run_preliminary("url") - - # Total cost should be accumulated (2.00) + current (1.00) = 3.00 - assert mock_post.call_args[0][5] == 3.00 - - -class TestRunThorough: - def test_full_success(self): - mock_comment = MagicMock() - mock_comment.id = 600 - - with patch("clayde.tasks.plan.get_github_client") as mock_gc, \ - patch("clayde.tasks.plan.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.plan.update_issue_state") as mock_update, \ - patch("clayde.tasks.plan.fetch_issue") as mock_fi, \ - patch("clayde.tasks.plan.get_default_branch", return_value="main"), \ - patch("clayde.tasks.plan.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.plan.get_issue_state", return_value={"preliminary_comment_id": 100}), \ - patch("clayde.tasks.plan.fetch_comment") as mock_fc, \ - patch("clayde.tasks.plan.fetch_issue_comments", return_value=[mock_comment]), \ - patch("clayde.tasks.plan.filter_comments", return_value=[]), \ - patch("clayde.tasks.plan._build_thorough_prompt", return_value="prompt"), \ - patch("clayde.tasks.plan.invoke_claude", return_value=_make_result( - '{"plan": "' + "x" * 250 + '"}', cost_eur=2.00)), \ - patch("clayde.tasks.plan._post_thorough_plan_comment", return_value=888) as mock_post, \ - patch("clayde.tasks.plan.pop_accumulated_cost", return_value=0.0): - mock_fc.return_value.body = "preliminary plan" - mock_fi.return_value.labels = [] - run_thorough("https://github.com/o/r/issues/1") - - calls = mock_update.call_args_list - assert calls[0][0] == ("https://github.com/o/r/issues/1", {"status": "planning"}) - last = calls[-1][0][1] - assert last["status"] == "awaiting_plan_approval" - assert last["plan_comment_id"] == 888 - assert "branch_name" not in last - # Cost is passed - assert mock_post.call_args[0][5] == 2.00 - - def test_usage_limit_sets_interrupted_and_accumulates_cost(self): - with patch("clayde.tasks.plan.get_github_client"), \ - patch("clayde.tasks.plan.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.plan.update_issue_state") as mock_update, \ - patch("clayde.tasks.plan.fetch_issue"), \ - patch("clayde.tasks.plan.get_default_branch", return_value="main"), \ - patch("clayde.tasks.plan.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.plan.get_issue_state", return_value={"preliminary_comment_id": 100}), \ - patch("clayde.tasks.plan.fetch_comment") as mock_fc, \ - patch("clayde.tasks.plan.fetch_issue_comments", return_value=[]), \ - patch("clayde.tasks.plan.filter_comments", return_value=[]), \ - patch("clayde.tasks.plan._build_thorough_prompt", return_value="prompt"), \ - patch("clayde.tasks.plan.invoke_claude", side_effect=UsageLimitError("limit", cost_eur=1.50)), \ - patch("clayde.tasks.plan.accumulate_cost") as mock_accum: - mock_fc.return_value.body = "plan" - run_thorough("url") - - last_call = mock_update.call_args_list[-1] - assert last_call[0][1]["status"] == "interrupted" - assert last_call[0][1]["interrupted_phase"] == "planning" - mock_accum.assert_called_once_with("url", 1.50) - - -class TestRunUpdate: - def test_updates_plan_and_posts_summary_with_cost(self): - new_comment = MagicMock() - new_comment.id = 300 - new_comment.user.login = "alice" - new_comment.body = "please change X" - - last_comment = MagicMock() - last_comment.id = 400 - - with patch("clayde.tasks.plan.get_github_client") as mock_gc, \ - patch("clayde.tasks.plan.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.plan.get_issue_state", return_value={ - "preliminary_comment_id": 100, - "last_seen_comment_id": 200, - }), \ - patch("clayde.tasks.plan.fetch_issue") as mock_fi, \ - patch("clayde.tasks.plan.get_default_branch", return_value="main"), \ - patch("clayde.tasks.plan.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.plan.fetch_comment") as mock_fc, \ - patch("clayde.tasks.plan.fetch_issue_comments", side_effect=[ - [new_comment], - [last_comment], - ]), \ - patch("clayde.tasks.plan.get_new_visible_comments", return_value=[new_comment]), \ - patch("clayde.tasks.plan.is_issue_visible", return_value=True), \ - patch("clayde.tasks.plan.invoke_claude", - return_value=_make_result('{"summary": "Summary", "updated_plan": "Updated plan text"}', cost_eur=0.80)), \ - patch("clayde.tasks.plan.edit_comment") as mock_edit, \ - patch("clayde.tasks.plan.post_comment") as mock_post, \ - patch("clayde.tasks.plan.update_issue_state") as mock_update, \ - patch("clayde.tasks.plan.pop_accumulated_cost", return_value=0.0): - mock_fc.return_value.body = "current plan" - mock_fi.return_value.body = "issue body" - mock_fi.return_value.title = "Title" - run_update("url", "preliminary") - - mock_edit.assert_called_once() - mock_post.assert_called_once() - posted_body = mock_post.call_args[0][4] - assert "Plan updated" in posted_body - assert "💸 This task cost 0.80€" in posted_body - - def test_usage_limit_accumulates_cost(self): - new_comment = MagicMock() - new_comment.id = 300 - new_comment.user.login = "alice" - new_comment.body = "change something" - - with patch("clayde.tasks.plan.get_github_client"), \ - patch("clayde.tasks.plan.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.plan.get_issue_state", return_value={ - "preliminary_comment_id": 100, - "last_seen_comment_id": 200, - }), \ - patch("clayde.tasks.plan.fetch_issue") as mock_fi, \ - patch("clayde.tasks.plan.get_default_branch", return_value="main"), \ - patch("clayde.tasks.plan.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.plan.fetch_comment") as mock_fc, \ - patch("clayde.tasks.plan.fetch_issue_comments", return_value=[new_comment]), \ - patch("clayde.tasks.plan.get_new_visible_comments", return_value=[new_comment]), \ - patch("clayde.tasks.plan.is_issue_visible", return_value=True), \ - patch("clayde.tasks.plan.invoke_claude", side_effect=UsageLimitError("limit", cost_eur=0.30)), \ - patch("clayde.tasks.plan.update_issue_state") as mock_update, \ - patch("clayde.tasks.plan.accumulate_cost") as mock_accum: - mock_fc.return_value.body = "plan" - mock_fi.return_value.body = "body" - mock_fi.return_value.title = "Title" - run_update("url", "preliminary") - - mock_accum.assert_called_once_with("url", 0.30) - # Status must NOT be changed — issue stays in awaiting_*_approval for retry - mock_update.assert_not_called() - - def test_skips_when_no_new_comments(self): - old_comment = MagicMock() - old_comment.id = 50 - old_comment.user.login = "alice" - - with patch("clayde.tasks.plan.get_github_client"), \ - patch("clayde.tasks.plan.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.plan.get_issue_state", return_value={ - "preliminary_comment_id": 100, - "last_seen_comment_id": 200, - }), \ - patch("clayde.tasks.plan.fetch_issue") as mock_fi, \ - patch("clayde.tasks.plan.get_default_branch", return_value="main"), \ - patch("clayde.tasks.plan.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.plan.fetch_comment") as mock_fc, \ - patch("clayde.tasks.plan.fetch_issue_comments", return_value=[old_comment]), \ - patch("clayde.tasks.plan.get_new_visible_comments", return_value=[]), \ - patch("clayde.tasks.plan.is_issue_visible", return_value=True), \ - patch("clayde.tasks.plan.invoke_claude") as mock_claude: - mock_fc.return_value.body = "plan" - mock_fi.return_value.body = "body" - mock_fi.return_value.title = "Title" - run_update("url", "preliminary") - - mock_claude.assert_not_called() - - def test_ignores_clayde_comments(self): - clayde_comment = MagicMock() - clayde_comment.id = 300 - clayde_comment.user.login = "ClaydeCode" - - with patch("clayde.tasks.plan.get_github_client"), \ - patch("clayde.tasks.plan.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.plan.get_issue_state", return_value={ - "preliminary_comment_id": 100, - "last_seen_comment_id": 200, - }), \ - patch("clayde.tasks.plan.fetch_issue") as mock_fi, \ - patch("clayde.tasks.plan.get_default_branch", return_value="main"), \ - patch("clayde.tasks.plan.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.plan.fetch_comment") as mock_fc, \ - patch("clayde.tasks.plan.fetch_issue_comments", return_value=[clayde_comment]), \ - patch("clayde.tasks.plan.get_new_visible_comments", return_value=[]), \ - patch("clayde.tasks.plan.is_issue_visible", return_value=True), \ - patch("clayde.tasks.plan.invoke_claude") as mock_claude: - mock_fc.return_value.body = "plan" - mock_fi.return_value.body = "body" - mock_fi.return_value.title = "Title" - run_update("url", "preliminary") - - mock_claude.assert_not_called() diff --git a/tests/test_tasks_review.py b/tests/test_tasks_review.py deleted file mode 100644 index 9af6dea..0000000 --- a/tests/test_tasks_review.py +++ /dev/null @@ -1,213 +0,0 @@ -"""Tests for clayde.tasks.review.""" - -from pathlib import Path -from unittest.mock import MagicMock, patch - -from clayde.claude import InvocationResult, UsageLimitError -from clayde.tasks.review import _build_prompt, _format_reviews, run - - -def _make_result(output: str, cost_eur: float = 0.50) -> InvocationResult: - """Helper to create an InvocationResult for testing.""" - return InvocationResult(output=output, cost_eur=cost_eur, input_tokens=100, output_tokens=50) - - -def _mock_settings(): - s = MagicMock() - s.github_username = "ClaydeCode" - return s - - -class TestFormatReviews: - def test_formats_review_with_body(self): - review = MagicMock() - review.id = 1 - review.user.login = "alice" - review.state = "CHANGES_REQUESTED" - review.body = "Please fix the typo" - - result = _format_reviews([review], []) - assert "@alice" in result - assert "CHANGES_REQUESTED" in result - assert "Please fix the typo" in result - - def test_formats_review_with_inline_comments(self): - review = MagicMock() - review.id = 1 - review.user.login = "alice" - review.state = "COMMENTED" - review.body = "" - - rc = MagicMock() - rc.pull_request_review_id = 1 - rc.path = "src/main.py" - rc.line = 42 - rc.body = "This line looks wrong" - - result = _format_reviews([review], [rc]) - assert "src/main.py" in result - assert "42" in result - assert "This line looks wrong" in result - - def test_empty_reviews(self): - result = _format_reviews([], []) - assert result == "(no review content)" - - -class TestBuildPrompt: - def test_renders_template(self): - issue = MagicMock() - issue.title = "Fix bug" - issue.body = "body" - prompt = _build_prompt(issue, "o", "r", 1, "/path", "clayde/issue-1", "review text") - assert "Fix bug" in prompt - assert "review text" in prompt - assert "clayde/issue-1" in prompt - - -class TestRun: - def test_no_reviews_does_nothing(self): - with patch("clayde.tasks.review.get_github_client") as mock_gc, \ - patch("clayde.tasks.review.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.review.get_issue_state", return_value={ - "pr_url": "https://github.com/o/r/pull/5", - "last_seen_review_id": 0, - }), \ - patch("clayde.tasks.review.get_settings", return_value=_mock_settings()), \ - patch("clayde.tasks.review.parse_pr_url", return_value=("o", "r", 5)), \ - patch("clayde.tasks.review.get_pr_reviews", return_value=[]), \ - patch("clayde.tasks.review.invoke_claude") as mock_claude: - run("url") - mock_claude.assert_not_called() - - def test_addresses_new_review_with_cost(self, tmp_path): - review = MagicMock() - review.id = 100 - review.user.login = "alice" - review.state = "CHANGES_REQUESTED" - review.body = "Please change X" - - review_comment = MagicMock() - review_comment.pull_request_review_id = 100 - review_comment.path = "src/file.py" - review_comment.line = 10 - review_comment.body = "Fix this line" - - with patch("clayde.tasks.review.get_github_client") as mock_gc, \ - patch("clayde.tasks.review.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.review.get_issue_state", return_value={ - "pr_url": "https://github.com/o/r/pull/5", - "last_seen_review_id": 0, - "branch_name": "clayde/issue-1", - }), \ - patch("clayde.tasks.review.get_settings", return_value=_mock_settings()), \ - patch("clayde.tasks.review.parse_pr_url", return_value=("o", "r", 5)), \ - patch("clayde.tasks.review.get_pr_reviews", return_value=[review]), \ - patch("clayde.tasks.review.get_pr_review_comments", return_value=[review_comment]), \ - patch("clayde.tasks.review.update_issue_state") as mock_update, \ - patch("clayde.tasks.review.fetch_issue") as mock_fi, \ - patch("clayde.tasks.review.get_default_branch", return_value="main"), \ - patch("clayde.tasks.review.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.review.invoke_claude", return_value=_make_result("Changes made", cost_eur=1.20)) as mock_claude, \ - patch("clayde.tasks.review.post_comment") as mock_post, \ - patch("clayde.tasks.review.pop_accumulated_cost", return_value=0.0), \ - patch("clayde.tasks.review.DATA_DIR", tmp_path): - mock_fi.return_value.title = "Fix bug" - mock_fi.return_value.body = "body" - run("url") - - mock_claude.assert_called_once() - mock_post.assert_called_once() - posted_body = mock_post.call_args[0][4] - assert "Review addressed" in posted_body - assert "💸 This task cost 1.20€" in posted_body - # Should return to pr_open - last_update = mock_update.call_args_list[-1][0][1] - assert last_update["status"] == "pr_open" - assert last_update["last_seen_review_id"] == 100 - - def test_approval_marks_done(self): - review = MagicMock() - review.id = 100 - review.user.login = "alice" - review.state = "APPROVED" - review.body = "" - - with patch("clayde.tasks.review.get_github_client"), \ - patch("clayde.tasks.review.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.review.get_issue_state", return_value={ - "pr_url": "https://github.com/o/r/pull/5", - "last_seen_review_id": 0, - }), \ - patch("clayde.tasks.review.get_settings", return_value=_mock_settings()), \ - patch("clayde.tasks.review.parse_pr_url", return_value=("o", "r", 5)), \ - patch("clayde.tasks.review.get_pr_reviews", return_value=[review]), \ - patch("clayde.tasks.review.get_pr_review_comments", return_value=[]), \ - patch("clayde.tasks.review.update_issue_state") as mock_update: - run("url") - - # Should update review id first, then set done - calls = mock_update.call_args_list - assert any(c[0][1].get("status") == "done" for c in calls) - - def test_usage_limit_sets_interrupted_and_accumulates_cost(self, tmp_path): - review = MagicMock() - review.id = 100 - review.user.login = "alice" - review.state = "CHANGES_REQUESTED" - review.body = "Fix it" - - with patch("clayde.tasks.review.get_github_client"), \ - patch("clayde.tasks.review.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.review.get_issue_state", return_value={ - "pr_url": "https://github.com/o/r/pull/5", - "last_seen_review_id": 0, - "branch_name": "clayde/issue-1", - }), \ - patch("clayde.tasks.review.get_settings", return_value=_mock_settings()), \ - patch("clayde.tasks.review.parse_pr_url", return_value=("o", "r", 5)), \ - patch("clayde.tasks.review.get_pr_reviews", return_value=[review]), \ - patch("clayde.tasks.review.get_pr_review_comments", return_value=[]), \ - patch("clayde.tasks.review.update_issue_state") as mock_update, \ - patch("clayde.tasks.review.fetch_issue") as mock_fi, \ - patch("clayde.tasks.review.get_default_branch", return_value="main"), \ - patch("clayde.tasks.review.ensure_repo", return_value="/tmp/repo"), \ - patch("clayde.tasks.review.invoke_claude", side_effect=UsageLimitError("limit", cost_eur=0.90)), \ - patch("clayde.tasks.review.accumulate_cost") as mock_accum, \ - patch("clayde.tasks.review.DATA_DIR", tmp_path): - mock_fi.return_value.title = "Fix bug" - mock_fi.return_value.body = "body" - run("url") - - last_call = mock_update.call_args_list[-1] - assert last_call[0][1]["status"] == "interrupted" - assert last_call[0][1]["interrupted_phase"] == "addressing_review" - mock_accum.assert_called_once_with("url", 0.90) - - def test_no_pr_url_skips(self): - with patch("clayde.tasks.review.get_github_client"), \ - patch("clayde.tasks.review.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.review.get_issue_state", return_value={}), \ - patch("clayde.tasks.review.invoke_claude") as mock_claude: - run("url") - mock_claude.assert_not_called() - - def test_ignores_own_reviews(self): - review = MagicMock() - review.id = 100 - review.user.login = "ClaydeCode" - review.state = "COMMENTED" - review.body = "My own review" - - with patch("clayde.tasks.review.get_github_client"), \ - patch("clayde.tasks.review.parse_issue_url", return_value=("o", "r", 1)), \ - patch("clayde.tasks.review.get_issue_state", return_value={ - "pr_url": "https://github.com/o/r/pull/5", - "last_seen_review_id": 0, - }), \ - patch("clayde.tasks.review.get_settings", return_value=_mock_settings()), \ - patch("clayde.tasks.review.parse_pr_url", return_value=("o", "r", 5)), \ - patch("clayde.tasks.review.get_pr_reviews", return_value=[review]), \ - patch("clayde.tasks.review.invoke_claude") as mock_claude: - run("url") - mock_claude.assert_not_called() diff --git a/tests/test_tasks_work.py b/tests/test_tasks_work.py new file mode 100644 index 0000000..85aa00c --- /dev/null +++ b/tests/test_tasks_work.py @@ -0,0 +1,274 @@ +"""Tests for clayde.tasks.work — unified work task.""" + +from unittest.mock import MagicMock, patch + +from clayde.claude import InvocationResult, InvocationTimeoutError, UsageLimitError +from clayde.tasks.work import _assign_reviewer, _format_reviews, run + + +def _make_result(output: str, cost_eur: float = 0.50) -> InvocationResult: + return InvocationResult(output=output, cost_eur=cost_eur, input_tokens=100, output_tokens=50) + + +def _mock_settings(github_username="ClaydeCode"): + s = MagicMock() + s.github_username = github_username + return s + + +class TestRun: + def _base_patches(self): + """Common patches for run() tests.""" + mock_issue = MagicMock() + mock_issue.title = "Test issue" + mock_issue.body = "Fix this thing" + mock_issue.labels = [] + mock_issue.user.login = "alice" + return mock_issue + + def test_posts_summary_on_success(self): + mock_issue = self._base_patches() + with patch("clayde.tasks.work.get_github_client"), \ + patch("clayde.tasks.work.parse_issue_url", return_value=("o", "r", 1)), \ + patch("clayde.tasks.work.get_issue_state", return_value={}), \ + patch("clayde.tasks.work.fetch_issue", return_value=mock_issue), \ + patch("clayde.tasks.work.get_default_branch", return_value="main"), \ + patch("clayde.tasks.work.ensure_repo", return_value="/tmp/repo"), \ + patch("clayde.tasks.work.fetch_issue_comments", return_value=[]), \ + patch("clayde.tasks.work.filter_comments", return_value=[]), \ + patch("clayde.tasks.work.is_issue_visible", return_value=True), \ + patch("clayde.tasks.work.render_template", return_value="prompt"), \ + patch("clayde.tasks.work.update_issue_state"), \ + patch("clayde.tasks.work.invoke_claude", + return_value=_make_result('{"summary": "Done the work"}')), \ + patch("clayde.tasks.work.find_open_pr", return_value=None), \ + patch("clayde.tasks.work.post_comment") as mock_post: + run("https://github.com/o/r/issues/1") + + mock_post.assert_called_once() + body = mock_post.call_args[0][4] + assert "Done the work" in body + + def test_propagates_usage_limit_error(self): + mock_issue = self._base_patches() + with patch("clayde.tasks.work.get_github_client"), \ + patch("clayde.tasks.work.parse_issue_url", return_value=("o", "r", 1)), \ + patch("clayde.tasks.work.get_issue_state", return_value={}), \ + patch("clayde.tasks.work.fetch_issue", return_value=mock_issue), \ + patch("clayde.tasks.work.get_default_branch", return_value="main"), \ + patch("clayde.tasks.work.ensure_repo", return_value="/tmp/repo"), \ + patch("clayde.tasks.work.fetch_issue_comments", return_value=[]), \ + patch("clayde.tasks.work.filter_comments", return_value=[]), \ + patch("clayde.tasks.work.is_issue_visible", return_value=True), \ + patch("clayde.tasks.work.render_template", return_value="prompt"), \ + patch("clayde.tasks.work.update_issue_state"), \ + patch("clayde.tasks.work.invoke_claude", + side_effect=UsageLimitError("limit", cost_eur=0.75)): + import pytest + with pytest.raises(UsageLimitError): + run("https://github.com/o/r/issues/1") + + def test_detects_and_persists_new_pr(self): + mock_issue = self._base_patches() + with patch("clayde.tasks.work.get_github_client"), \ + patch("clayde.tasks.work.parse_issue_url", return_value=("o", "r", 1)), \ + patch("clayde.tasks.work.get_issue_state", return_value={}), \ + patch("clayde.tasks.work.fetch_issue", return_value=mock_issue), \ + patch("clayde.tasks.work.get_default_branch", return_value="main"), \ + patch("clayde.tasks.work.ensure_repo", return_value="/tmp/repo"), \ + patch("clayde.tasks.work.fetch_issue_comments", return_value=[]), \ + patch("clayde.tasks.work.filter_comments", return_value=[]), \ + patch("clayde.tasks.work.is_issue_visible", return_value=True), \ + patch("clayde.tasks.work.render_template", return_value="prompt"), \ + patch("clayde.tasks.work.update_issue_state") as mock_update, \ + patch("clayde.tasks.work.invoke_claude", + return_value=_make_result('{"summary": "Implemented"}')), \ + patch("clayde.tasks.work.find_open_pr", + return_value="https://github.com/o/r/pull/5"), \ + patch("clayde.tasks.work.post_comment"), \ + patch("clayde.tasks.work._assign_reviewer"): + run("https://github.com/o/r/issues/1") + + # pr_url should be persisted + pr_update = next( + (c[0][1] for c in mock_update.call_args_list if "pr_url" in c[0][1]), + None, + ) + assert pr_update is not None + assert pr_update["pr_url"] == "https://github.com/o/r/pull/5" + + def test_assigns_reviewer_for_new_pr(self): + mock_issue = self._base_patches() + with patch("clayde.tasks.work.get_github_client"), \ + patch("clayde.tasks.work.parse_issue_url", return_value=("o", "r", 1)), \ + patch("clayde.tasks.work.get_issue_state", return_value={}), \ + patch("clayde.tasks.work.fetch_issue", return_value=mock_issue), \ + patch("clayde.tasks.work.get_default_branch", return_value="main"), \ + patch("clayde.tasks.work.ensure_repo", return_value="/tmp/repo"), \ + patch("clayde.tasks.work.fetch_issue_comments", return_value=[]), \ + patch("clayde.tasks.work.filter_comments", return_value=[]), \ + patch("clayde.tasks.work.is_issue_visible", return_value=True), \ + patch("clayde.tasks.work.render_template", return_value="prompt"), \ + patch("clayde.tasks.work.update_issue_state"), \ + patch("clayde.tasks.work.invoke_claude", + return_value=_make_result('{"summary": "Implemented"}')), \ + patch("clayde.tasks.work.find_open_pr", + return_value="https://github.com/o/r/pull/5"), \ + patch("clayde.tasks.work.post_comment"), \ + patch("clayde.tasks.work._assign_reviewer") as mock_assign: + run("https://github.com/o/r/issues/1") + + mock_assign.assert_called_once() + + def test_does_not_reassign_reviewer_for_existing_pr(self): + mock_issue = self._base_patches() + existing_pr = "https://github.com/o/r/pull/5" + with patch("clayde.tasks.work.get_github_client"), \ + patch("clayde.tasks.work.parse_issue_url", return_value=("o", "r", 1)), \ + patch("clayde.tasks.work.get_issue_state", + return_value={"pr_url": existing_pr}), \ + patch("clayde.tasks.work.fetch_issue", return_value=mock_issue), \ + patch("clayde.tasks.work.get_default_branch", return_value="main"), \ + patch("clayde.tasks.work.ensure_repo", return_value="/tmp/repo"), \ + patch("clayde.tasks.work.fetch_issue_comments", return_value=[]), \ + patch("clayde.tasks.work.filter_comments", return_value=[]), \ + patch("clayde.tasks.work.is_issue_visible", return_value=True), \ + patch("clayde.tasks.work.get_pr_reviews", return_value=[]), \ + patch("clayde.tasks.work.get_pr_review_comments", return_value=[]), \ + patch("clayde.tasks.work.parse_pr_url", return_value=("o", "r", 5)), \ + patch("clayde.tasks.work.render_template", return_value="prompt"), \ + patch("clayde.tasks.work.update_issue_state"), \ + patch("clayde.tasks.work.invoke_claude", + return_value=_make_result('{"summary": "Review addressed"}')), \ + patch("clayde.tasks.work.find_open_pr", return_value=existing_pr), \ + patch("clayde.tasks.work.post_comment"), \ + patch("clayde.tasks.work._assign_reviewer") as mock_assign: + run("https://github.com/o/r/issues/1") + + mock_assign.assert_not_called() + + def test_falls_back_to_raw_output_on_json_parse_failure(self): + mock_issue = self._base_patches() + with patch("clayde.tasks.work.get_github_client"), \ + patch("clayde.tasks.work.parse_issue_url", return_value=("o", "r", 1)), \ + patch("clayde.tasks.work.get_issue_state", return_value={}), \ + patch("clayde.tasks.work.fetch_issue", return_value=mock_issue), \ + patch("clayde.tasks.work.get_default_branch", return_value="main"), \ + patch("clayde.tasks.work.ensure_repo", return_value="/tmp/repo"), \ + patch("clayde.tasks.work.fetch_issue_comments", return_value=[]), \ + patch("clayde.tasks.work.filter_comments", return_value=[]), \ + patch("clayde.tasks.work.is_issue_visible", return_value=True), \ + patch("clayde.tasks.work.render_template", return_value="prompt"), \ + patch("clayde.tasks.work.update_issue_state"), \ + patch("clayde.tasks.work.invoke_claude", + return_value=_make_result("I just posted a question on the issue.")), \ + patch("clayde.tasks.work.find_open_pr", return_value=None), \ + patch("clayde.tasks.work.post_comment") as mock_post: + run("https://github.com/o/r/issues/1") + + mock_post.assert_called_once() + body = mock_post.call_args[0][4] + assert "I just posted a question" in body + + def test_filters_invisible_issue_body(self): + mock_issue = self._base_patches() + mock_issue.user.login = "unknown" # not whitelisted + + captured_prompt = {} + + def capture_render(**kwargs): + captured_prompt.update(kwargs) + return "prompt" + + with patch("clayde.tasks.work.get_github_client"), \ + patch("clayde.tasks.work.parse_issue_url", return_value=("o", "r", 1)), \ + patch("clayde.tasks.work.get_issue_state", return_value={}), \ + patch("clayde.tasks.work.fetch_issue", return_value=mock_issue), \ + patch("clayde.tasks.work.get_default_branch", return_value="main"), \ + patch("clayde.tasks.work.ensure_repo", return_value="/tmp/repo"), \ + patch("clayde.tasks.work.fetch_issue_comments", return_value=[]), \ + patch("clayde.tasks.work.filter_comments", return_value=[]), \ + patch("clayde.tasks.work.is_issue_visible", return_value=False), \ + patch("clayde.tasks.work.render_template", + side_effect=lambda name, **kw: (captured_prompt.update(kw), "prompt")[1]), \ + patch("clayde.tasks.work.update_issue_state"), \ + patch("clayde.tasks.work.invoke_claude", + return_value=_make_result('{"summary": "done"}')), \ + patch("clayde.tasks.work.find_open_pr", return_value=None), \ + patch("clayde.tasks.work.post_comment"): + run("https://github.com/o/r/issues/1") + + assert captured_prompt.get("body") == "(filtered)" + + +class TestFormatReviews: + def test_formats_review_with_body(self): + review = MagicMock() + review.id = 1 + review.user.login = "alice" + review.state = "CHANGES_REQUESTED" + review.body = "Please fix the typo" + + result = _format_reviews([review], []) + assert "@alice" in result + assert "CHANGES_REQUESTED" in result + assert "Please fix the typo" in result + + def test_formats_review_with_inline_comments(self): + review = MagicMock() + review.id = 1 + review.user.login = "alice" + review.state = "COMMENTED" + review.body = "" + + rc = MagicMock() + rc.pull_request_review_id = 1 + rc.path = "src/main.py" + rc.line = 42 + rc.body = "This line looks wrong" + + result = _format_reviews([review], [rc]) + assert "src/main.py" in result + assert "42" in result + assert "This line looks wrong" in result + + def test_returns_none_placeholder_for_empty(self): + result = _format_reviews([], []) + assert result == "(none)" + + def test_skips_review_with_no_body_and_no_comments(self): + review = MagicMock() + review.id = 1 + review.user.login = "alice" + review.state = "APPROVED" + review.body = "" + + result = _format_reviews([review], []) + assert result == "(none)" + + +class TestAssignReviewer: + def test_assigns_non_self_author(self): + g = MagicMock() + with patch("clayde.tasks.work.parse_pr_url", return_value=("o", "r", 5)), \ + patch("clayde.tasks.work.get_issue_author", return_value="alice"), \ + patch("clayde.tasks.work.get_settings", return_value=_mock_settings()), \ + patch("clayde.tasks.work.add_pr_reviewer") as mock_add: + _assign_reviewer(g, "o", "r", 1, "https://github.com/o/r/pull/5") + mock_add.assert_called_once_with(g, "o", "r", 5, "alice") + + def test_skips_self_author(self): + g = MagicMock() + with patch("clayde.tasks.work.parse_pr_url", return_value=("o", "r", 5)), \ + patch("clayde.tasks.work.get_issue_author", return_value="claydecode"), \ + patch("clayde.tasks.work.get_settings", + return_value=_mock_settings(github_username="ClaydeCode")), \ + patch("clayde.tasks.work.add_pr_reviewer") as mock_add: + _assign_reviewer(g, "o", "r", 1, "https://github.com/o/r/pull/5") + mock_add.assert_not_called() + + def test_tolerates_exception(self): + g = MagicMock() + with patch("clayde.tasks.work.parse_pr_url", side_effect=Exception("bad url")): + # Should not raise + _assign_reviewer(g, "o", "r", 1, "https://github.com/o/r/pull/5")