diff --git a/CLAUDE.md b/CLAUDE.md index 33e094a8..ba039e73 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -122,6 +122,27 @@ podman rm $(podman ps -a --filter name=forge- -q) Skip-gate commands are only active at CI stages (`wait_for_ci_gate`, `ci_evaluator`, `attempt_ci_fix`). Rebase works from any workflow stage. +## PRD Approval via GitHub PR + +Opt-in per project via Jira project property. When configured, Forge opens a PR in the proposals repo instead of posting the PRD to Jira. Reviewer feedback triggers regeneration; merging the PR signals approval. + +**Per-project config (Jira project property):** + +| Property | Example | Description | +|----------|---------|-------------| +| `forge.prd_proposals_repo` | `org/enhancement-proposals` | Enables PR-based PRD approval for this project | + +Set via: `jira project-property set forge.prd_proposals_repo "owner/repo"` + +**Global fallbacks (`.env`, used when `FORGE_REQUIRE_PROJECT_CONFIG=false`):** + +| Setting | Default | Description | +|---------|---------|-------------| +| `PRD_PROPOSALS_REPO` | (empty) | Fallback `owner/repo` for projects without the property | +| `PRD_PROPOSALS_PATH` | `proposals` | Directory in the repo for PRD files | + +Branch naming convention: `forge/prd/{ticket-key}` (e.g., `forge/prd/proj-123`). + ## Container Execution Tasks are implemented in ephemeral Podman containers: diff --git a/src/forge/config.py b/src/forge/config.py index f378727a..0a10ea17 100644 --- a/src/forge/config.py +++ b/src/forge/config.py @@ -115,6 +115,21 @@ def atlassian_auth_base64(self) -> str: ), ) + # PRD Approval Configuration (global fallbacks — per-project config via + # Jira project property forge.prd_proposals_repo takes precedence) + prd_proposals_repo: str = Field( + default="", + description=( + "Global fallback GitHub repo (owner/repo) for enhancement proposals. " + "Per-project config via Jira project property forge.prd_proposals_repo " + "takes precedence. Only used when forge_require_project_config is False." + ), + ) + prd_proposals_path: str = Field( + default="proposals", + description="Directory in the proposals repo where PRD files are stored.", + ) + @property def known_repos(self) -> list[str]: """Get list of known repositories.""" diff --git a/src/forge/integrations/github/client.py b/src/forge/integrations/github/client.py index a3ae3301..b1a1f98d 100644 --- a/src/forge/integrations/github/client.py +++ b/src/forge/integrations/github/client.py @@ -695,3 +695,114 @@ async def get_fork_owner(self) -> str: return self.settings.github_fork_owner user = await self.get_authenticated_user() return user["login"] + + async def create_branch( + self, + owner: str, + repo: str, + branch_name: str, + base: str = "main", + ) -> dict[str, Any]: + """Create a new branch from a base ref. + + Args: + owner: Repository owner. + repo: Repository name. + branch_name: New branch name. + base: Base branch to branch from. + + Returns: + API response with ref details. + """ + client = await self._get_client() + + ref_response = await client.get(f"/repos/{owner}/{repo}/git/ref/heads/{base}") + ref_response.raise_for_status() + sha = ref_response.json()["object"]["sha"] + + try: + response = await client.post( + f"/repos/{owner}/{repo}/git/refs", + json={"ref": f"refs/heads/{branch_name}", "sha": sha}, + ) + response.raise_for_status() + data = response.json() + logger.info(f"Created branch {branch_name} in {owner}/{repo}") + return data + except httpx.HTTPStatusError as e: + if e.response.status_code == 422: + logger.info(f"Branch {branch_name} already exists in {owner}/{repo}") + return {"ref": f"refs/heads/{branch_name}", "object": {"sha": sha}} + raise + + async def create_or_update_file( + self, + owner: str, + repo: str, + path: str, + content: str, + message: str, + branch: str, + sha: str | None = None, + ) -> dict[str, Any]: + """Create or update a file via the Contents API. + + Args: + owner: Repository owner. + repo: Repository name. + path: File path in the repository. + content: File content (plain text, will be base64-encoded). + message: Commit message. + branch: Target branch. + sha: Existing file SHA (required for updates, omit for creates). + + Returns: + API response with content details. + """ + import base64 as b64 + + client = await self._get_client() + body: dict[str, Any] = { + "message": message, + "content": b64.b64encode(content.encode()).decode(), + "branch": branch, + } + if sha: + body["sha"] = sha + + response = await client.put(f"/repos/{owner}/{repo}/contents/{path}", json=body) + response.raise_for_status() + data = response.json() + logger.info(f"{'Updated' if sha else 'Created'} file {path} on {branch} in {owner}/{repo}") + return data + + async def get_file_contents( + self, + owner: str, + repo: str, + path: str, + ref: str, + ) -> dict[str, Any] | None: + """Get file contents and metadata from a repository. + + Args: + owner: Repository owner. + repo: Repository name. + path: File path in the repository. + ref: Git ref (branch, tag, or SHA). + + Returns: + File metadata including sha, or None if not found. + """ + client = await self._get_client() + try: + response = await client.get( + f"/repos/{owner}/{repo}/contents/{path}", + params={"ref": ref}, + ) + response.raise_for_status() + return response.json() + except httpx.HTTPStatusError as e: + if e.response.status_code == 404: + return None + raise diff --git a/src/forge/integrations/jira/client.py b/src/forge/integrations/jira/client.py index 87bae8be..0545e914 100644 --- a/src/forge/integrations/jira/client.py +++ b/src/forge/integrations/jira/client.py @@ -943,6 +943,29 @@ async def get_project_default_repo(self, project_key: str) -> str: logger.info(f"Project {project_key}: default repo: {value}") return value + async def get_prd_proposals_repo(self, project_key: str) -> str | None: + """Fetch the forge.prd_proposals_repo project property. + + When set, enables PRD approval via GitHub PR for this project. + The value is a GitHub repo in "owner/repo" format. + + Args: + project_key: The Jira project key. + + Returns: + Repo string in "owner/repo" format, or None if not configured. + """ + value = await self.get_project_property(project_key, "forge.prd_proposals_repo") + if value is None: + return None + if not isinstance(value, str) or "/" not in value: + logger.warning( + f"forge.prd_proposals_repo for project {project_key} is malformed: {value!r}" + ) + return None + logger.info(f"Project {project_key}: PRD proposals repo: {value}") + return value + async def get_skills_config(self, project_key: str) -> list[SkillEntry] | None: """Fetch and parse the forge.skills project property. diff --git a/src/forge/orchestrator/worker.py b/src/forge/orchestrator/worker.py index 0e82003b..9b0fc80f 100644 --- a/src/forge/orchestrator/worker.py +++ b/src/forge/orchestrator/worker.py @@ -21,7 +21,7 @@ from forge.integrations.github.client import GitHubClient from forge.integrations.jira.client import JiraClient from forge.models.events import EventSource -from forge.models.workflow import TicketType +from forge.models.workflow import ForgeLabel, TicketType from forge.orchestrator.checkpointer import get_checkpointer, get_ticket_from_pr_index from forge.queue.consumer import QueueConsumer from forge.queue.models import QueueMessage @@ -39,6 +39,8 @@ def _is_workflow_errored(state: dict) -> bool: return not state.get("is_paused") and state.get("last_error") is not None +_PRD_GATE_NODES = ("prd_approval_gate", "generate_prd", "regenerate_prd") + # Matches >option N anywhere in comment (case-insensitive, first match wins) # Supports both start-of-line usage (>option 2) and in-prose usage (let's go with >option 2) _OPTION_PATTERN = re.compile(r"(?mi)>option\s+(\d+)") @@ -150,6 +152,23 @@ async def _resolve_ticket_from_pr_index(self, message: QueueMessage) -> QueueMes return message + def _is_prd_pr_event(self, message: QueueMessage, current_state: dict[str, Any]) -> bool: + """Check if a GitHub event targets the PRD proposals PR.""" + if message.source != EventSource.GITHUB: + return False + prd_pr_number = current_state.get("prd_pr_number") + prd_pr_repo = current_state.get("prd_pr_repo") + if not prd_pr_number or not prd_pr_repo: + return False + + payload = message.payload + repo_full = payload.get("repository", {}).get("full_name", "") + event_pr_number = payload.get("pull_request", {}).get("number") or payload.get( + "issue", {} + ).get("number") + + return repo_full == prd_pr_repo and event_pr_number == prd_pr_number + async def _process_workflow(self, message: QueueMessage) -> None: """Process a message through the workflow. @@ -558,8 +577,16 @@ async def _handle_resume_event( # Check for rejection comment (contains feedback) # Determine if comment is on Epic/Task (child) vs Feature (parent) # based on current workflow phase + # + # Skip Jira comment feedback when PRD review happens on a GitHub PR — + # feedback should come from the PR, not Jira. comment_ticket_key = None comment_ticket_type = None # "epic" or "task" + if comment and current_state.get("prd_pr_number") and current_node in _PRD_GATE_NODES: + logger.info( + f"Ignoring Jira comment for {message.ticket_key} — PRD review is on GitHub PR" + ) + comment = {} if comment: comment_body = comment.get("body", "") # Extract text from ADF if needed @@ -690,6 +717,98 @@ async def _handle_resume_event( else: logger.info(f"Detected Feature-level comment: {feedback[:100]}...") + # GitHub events targeting the PRD proposals PR — handled at prd_approval_gate. + # Merge = approval. Review with feedback = revision. Comment = feedback/question. + if self._is_prd_pr_event(message, current_state) and current_node in _PRD_GATE_NODES: + event = message.event_type + + if "pull_request_review" in event: + review = payload.get("review", {}) + review_state = review.get("state", "").lower() + review_body = review.get("body", "") or "" + + # Merge-only approval: review approval is intentionally ignored + if review_state in ("changes_requested", "commented"): + repo_full = payload.get("repository", {}).get("full_name", "") + pr_number = payload.get("pull_request", {}).get("number") + inline_comments = [] + if repo_full and pr_number: + _owner, _repo = repo_full.split("/", 1) + gh = GitHubClient() + try: + inline_comments = await gh.get_pull_request_review_comments( + _owner, _repo, pr_number + ) + finally: + await gh.close() + + parts = [] + if review_body.strip(): + parts.append(review_body.strip()) + if inline_comments: + inline_text = "\n\n".join( + f"**{c['path']}** (line {c['position']}):\n{c['body']}" + for c in inline_comments + ) + parts.append(f"Inline comments:\n{inline_text}") + + if parts: + feedback = "\n\n".join(parts) + is_rejected = True + logger.info( + f"PRD PR review ({review_state}) for {message.ticket_key}: " + f"body={'yes' if review_body.strip() else 'no'}, " + f"inline={len(inline_comments)}" + ) + else: + logger.info( + f"PRD PR review ({review_state}) for {message.ticket_key} " + "with no content — ignoring" + ) + return current_state + + elif "pull_request" in event and payload.get("pull_request", {}).get("merged") is True: + is_approved = True + logger.info(f"PRD PR merged for {message.ticket_key}") + # Sync Jira label + jira = JiraClient() + try: + await jira.set_workflow_label(message.ticket_key, ForgeLabel.PRD_APPROVED) + finally: + await jira.close() + + elif "issue_comment" in event: + gh_comment = payload.get("comment", {}) + comment_body = gh_comment.get("body", "").strip() + sender_login = payload.get("sender", {}).get("login", "") + + if comment_body and sender_login: + # Skip self-comments + gh = GitHubClient() + try: + forge_user = await gh.get_authenticated_user() + forge_login = forge_user.get("login", "") + finally: + await gh.close() + + if sender_login == forge_login: + logger.debug(f"Ignoring self-comment on PRD PR for {message.ticket_key}") + return current_state + + comment_type = classify_comment(comment_body) + if comment_type == CommentType.QUESTION: + is_question = True + feedback = comment_body + logger.info( + f"PRD PR question for {message.ticket_key}: {comment_body[:100]}..." + ) + else: + is_rejected = True + feedback = comment_body + logger.info( + f"PRD PR feedback for {message.ticket_key}: {comment_body[:100]}..." + ) + # GitHub pull_request_review events — handled when at human_review_gate. # A review submission is the primary signal for the human review stage. if ( diff --git a/src/forge/workflow/feature/state.py b/src/forge/workflow/feature/state.py index 5a85a0e7..73ed3226 100644 --- a/src/forge/workflow/feature/state.py +++ b/src/forge/workflow/feature/state.py @@ -44,6 +44,13 @@ class FeatureState( generation_context: dict[str, Any] # Stored context from generation is_question: bool # Current comment is a question (not feedback) + # PRD PR tracking (enhancement proposal flow) + prd_pr_url: str | None + prd_pr_number: int | None + prd_pr_repo: str | None + prd_pr_branch: str | None + prd_pr_file_path: str | None + def create_initial_feature_state(ticket_key: str, **kwargs: Any) -> FeatureState: """Create initial state for a new Feature workflow run.""" @@ -103,6 +110,11 @@ def create_initial_feature_state(ticket_key: str, **kwargs: Any) -> FeatureState "qa_history": [], "generation_context": {}, "is_question": False, + "prd_pr_url": None, + "prd_pr_number": None, + "prd_pr_repo": None, + "prd_pr_branch": None, + "prd_pr_file_path": None, } # Merge with kwargs, letting kwargs override defaults diff --git a/src/forge/workflow/nodes/prd_generation.py b/src/forge/workflow/nodes/prd_generation.py index 2a7b20d1..c957fc70 100644 --- a/src/forge/workflow/nodes/prd_generation.py +++ b/src/forge/workflow/nodes/prd_generation.py @@ -1,19 +1,146 @@ """PRD generation node for LangGraph workflow.""" import logging +import re from datetime import UTC, datetime from typing import Any from forge.config import get_settings from forge.integrations.agents import ForgeAgent +from forge.integrations.github.client import GitHubClient from forge.integrations.jira.client import JiraClient from forge.models.workflow import ForgeLabel +from forge.orchestrator.checkpointer import set_pr_ticket_index from forge.workflow.feature.state import FeatureState as WorkflowState from forge.workflow.utils import update_state_timestamp logger = logging.getLogger(__name__) +def _slugify(text: str, max_length: int = 60) -> str: + """Convert text to URL-safe slug.""" + slug = text.lower().strip() + slug = re.sub(r"[^\w\s-]", "", slug) + slug = re.sub(r"[\s_]+", "-", slug) + slug = re.sub(r"-+", "-", slug).strip("-") + return slug[:max_length] + + +async def _resolve_prd_proposals_repo(project_key: str, jira: JiraClient) -> str | None: + """Resolve the PRD proposals repo for a project. + + Checks the Jira project property first (forge.prd_proposals_repo), + then falls back to the global env var when project config is not required. + + Returns: + Repo string in "owner/repo" format, or None if not configured. + """ + proposals_repo = await jira.get_prd_proposals_repo(project_key) + if proposals_repo: + return proposals_repo + + settings = get_settings() + if not settings.forge_require_project_config and settings.prd_proposals_repo: + logger.info( + f"Project {project_key}: using global fallback PRD proposals repo: " + f"{settings.prd_proposals_repo}" + ) + return settings.prd_proposals_repo + + return None + + +async def _create_prd_proposal_pr( + ticket_key: str, + prd_content: str, + summary: str, + proposals_repo: str, +) -> dict[str, Any]: + """Create a PR with the PRD in the enhancement proposals repo.""" + settings = get_settings() + owner, repo = proposals_repo.split("/", 1) + branch = f"forge/prd/{ticket_key.lower()}" + file_path = f"{settings.prd_proposals_path}/{ticket_key}-{_slugify(summary)}.md" + + gh = GitHubClient() + jira = JiraClient() + try: + await gh.create_branch(owner, repo, branch) + await gh.create_or_update_file( + owner=owner, + repo=repo, + path=file_path, + content=prd_content, + message=f"Add PRD for {ticket_key}", + branch=branch, + ) + pr_data = await gh.create_pull_request( + owner=owner, + repo=repo, + title=f"[{ticket_key}] PRD: {summary}", + body=prd_content, + head=branch, + ) + + pr_url = pr_data["html_url"] + pr_number = pr_data["number"] + + await set_pr_ticket_index(pr_url, ticket_key) + await jira.set_workflow_label(ticket_key, ForgeLabel.PRD_PENDING) + await jira.add_comment( + ticket_key, + f"PRD published for review: {pr_url}", + ) + + return { + "prd_pr_url": pr_url, + "prd_pr_number": pr_number, + "prd_pr_repo": proposals_repo, + "prd_pr_branch": branch, + "prd_pr_file_path": file_path, + } + finally: + await gh.close() + await jira.close() + + +async def _update_prd_proposal_pr( + ticket_key: str, + prd_content: str, + state: dict[str, Any], +) -> None: + """Push updated PRD content to the existing proposal PR branch.""" + owner, repo = state["prd_pr_repo"].split("/", 1) + branch = state["prd_pr_branch"] + pr_number = state["prd_pr_number"] + file_path = state["prd_pr_file_path"] + + gh = GitHubClient() + try: + file_meta = await gh.get_file_contents(owner, repo, file_path, branch) + if not file_meta: + logger.warning(f"Could not find PRD file {file_path} on branch {branch}") + return + + await gh.create_or_update_file( + owner=owner, + repo=repo, + path=file_path, + content=prd_content, + message=f"Revise PRD for {ticket_key} based on feedback", + branch=branch, + sha=file_meta["sha"], + ) + await gh.create_issue_comment( + owner, + repo, + pr_number, + "PRD has been revised based on feedback. Please review the updated version.", + ) + finally: + await gh.close() + + async def generate_prd(state: WorkflowState) -> WorkflowState: """Generate a PRD from raw requirements in Jira description. @@ -60,27 +187,33 @@ async def generate_prd(state: WorkflowState) -> WorkflowState: # Generate PRD using Claude - primary operation prd_content = await agent.generate_prd(raw_requirements, context) - # Update Jira with generated PRD - secondary operation + # Publish PRD - either as GitHub PR or Jira update + # Per-project opt-in: check forge.prd_proposals_repo project property + proposals_repo = await _resolve_prd_proposals_repo(issue.project_key, jira) + settings = get_settings() + prd_pr_result = None try: - settings = get_settings() - if settings.jira_store_in_comments: - # Store PRD in a structured comment - await jira.add_structured_comment( - ticket_key, - "Product Requirements Document (PRD)", - prd_content, - comment_type="prd", + if proposals_repo: + prd_pr_result = await _create_prd_proposal_pr( + ticket_key=ticket_key, + prd_content=prd_content, + summary=issue.summary, + proposals_repo=proposals_repo, ) else: - # Update description directly - await jira.update_description(ticket_key, prd_content) - - # Set workflow label (instead of custom status transition) - await jira.set_workflow_label(ticket_key, ForgeLabel.PRD_PENDING) + if settings.jira_store_in_comments: + await jira.add_structured_comment( + ticket_key, + "Product Requirements Document (PRD)", + prd_content, + comment_type="prd", + ) + else: + await jira.update_description(ticket_key, prd_content) + await jira.set_workflow_label(ticket_key, ForgeLabel.PRD_PENDING) except Exception as e: - # Jira update failed but we have content - log and continue jira_error = str(e) - logger.warning(f"Jira update failed for {ticket_key}, but PRD was generated: {e}") + logger.warning(f"PRD publish failed for {ticket_key}: {e}") logger.info(f"PRD generated for {ticket_key} ({len(prd_content)} chars)") @@ -92,16 +225,19 @@ async def generate_prd(state: WorkflowState) -> WorkflowState: "generated_at": datetime.now(UTC).isoformat(), } - # If Jira failed, set a warning but still advance (content exists) - return update_state_timestamp( + # If publish failed, set a warning but still advance (content exists) + result = update_state_timestamp( { **state, "prd_content": prd_content, "generation_context": generation_context, "current_node": "prd_approval_gate", - "last_error": f"Jira update pending: {jira_error}" if jira_error else None, + "last_error": f"PRD publish pending: {jira_error}" if jira_error else None, } ) + if prd_pr_result: + result.update(prd_pr_result) + return result except Exception as e: logger.error(f"PRD generation failed for {ticket_key}: {e}") @@ -158,23 +294,24 @@ async def regenerate_prd_with_feedback(state: WorkflowState) -> WorkflowState: ticket_key=ticket_key, ) - # Update Jira with regenerated PRD - settings = get_settings() - if settings.jira_store_in_comments: - await jira.add_structured_comment( + # Publish revised PRD + if state.get("prd_pr_number"): + await _update_prd_proposal_pr(ticket_key, new_prd, state) + else: + settings = get_settings() + if settings.jira_store_in_comments: + await jira.add_structured_comment( + ticket_key, + "Product Requirements Document (PRD)", + new_prd, + comment_type="prd", + ) + else: + await jira.update_description(ticket_key, new_prd) + await jira.add_comment( ticket_key, - "Product Requirements Document (PRD)", - new_prd, - comment_type="prd", + "PRD has been revised based on feedback. Please review.", ) - else: - await jira.update_description(ticket_key, new_prd) - - # Add comment acknowledging the revision - await jira.add_comment( - ticket_key, - "PRD has been revised based on feedback. Please review.", - ) logger.info(f"PRD regenerated for {ticket_key} ({len(new_prd)} chars)") diff --git a/src/forge/workflow/utils/comment_classifier.py b/src/forge/workflow/utils/comment_classifier.py index ef339240..9271bcae 100644 --- a/src/forge/workflow/utils/comment_classifier.py +++ b/src/forge/workflow/utils/comment_classifier.py @@ -19,7 +19,7 @@ class CommentType(StrEnum): def classify_comment(comment_text: str) -> CommentType: - """Classify a Jira comment into question or feedback. + """Classify a comment into question or feedback. Classification rules: - Questions: Comments starting with '?' or '@forge ask' (case-insensitive) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 972c0d39..9307a3a2 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -64,6 +64,7 @@ def mock_jira_client() -> MagicMock: mock.set_workflow_label = AsyncMock() mock.add_structured_comment = AsyncMock() mock.get_structured_comment = AsyncMock(return_value=None) + mock.get_prd_proposals_repo = AsyncMock(return_value=None) mock.add_attachment = AsyncMock(return_value={"id": "12345"}) mock.close = AsyncMock() return mock diff --git a/tests/unit/integrations/github/test_content_api.py b/tests/unit/integrations/github/test_content_api.py new file mode 100644 index 00000000..a7b4fa05 --- /dev/null +++ b/tests/unit/integrations/github/test_content_api.py @@ -0,0 +1,184 @@ +"""Tests for GitHub Content API methods.""" + +import base64 +from unittest.mock import AsyncMock, MagicMock + +import httpx +import pytest + +from forge.integrations.github.client import GitHubClient + + +@pytest.fixture +def github_client(mock_settings): + client = GitHubClient(settings=mock_settings) + client._client = AsyncMock(spec=httpx.AsyncClient) + client._client.is_closed = False + return client + + +class TestCreateBranch: + @pytest.mark.asyncio + async def test_creates_branch_from_main(self, github_client): + mock_client = github_client._client + + ref_response = MagicMock() + ref_response.json.return_value = {"object": {"sha": "abc123"}} + ref_response.raise_for_status = MagicMock() + + create_response = MagicMock() + create_response.json.return_value = {"ref": "refs/heads/forge/prd/test-123"} + create_response.raise_for_status = MagicMock() + + mock_client.get = AsyncMock(return_value=ref_response) + mock_client.post = AsyncMock(return_value=create_response) + + result = await github_client.create_branch("owner", "repo", "forge/prd/test-123") + + mock_client.get.assert_called_once_with("/repos/owner/repo/git/ref/heads/main") + mock_client.post.assert_called_once_with( + "/repos/owner/repo/git/refs", + json={"ref": "refs/heads/forge/prd/test-123", "sha": "abc123"}, + ) + assert result["ref"] == "refs/heads/forge/prd/test-123" + + @pytest.mark.asyncio + async def test_handles_branch_already_exists(self, github_client): + mock_client = github_client._client + + ref_response = MagicMock() + ref_response.json.return_value = {"object": {"sha": "abc123"}} + ref_response.raise_for_status = MagicMock() + + error_response = MagicMock() + error_response.status_code = 422 + error_response.raise_for_status = MagicMock( + side_effect=httpx.HTTPStatusError( + "Unprocessable", request=MagicMock(), response=error_response + ) + ) + + mock_client.get = AsyncMock(return_value=ref_response) + mock_client.post = AsyncMock(return_value=error_response) + + result = await github_client.create_branch("owner", "repo", "forge/prd/test-123") + assert result is not None + + @pytest.mark.asyncio + async def test_creates_branch_from_custom_base(self, github_client): + mock_client = github_client._client + + ref_response = MagicMock() + ref_response.json.return_value = {"object": {"sha": "def456"}} + ref_response.raise_for_status = MagicMock() + + create_response = MagicMock() + create_response.json.return_value = {"ref": "refs/heads/my-branch"} + create_response.raise_for_status = MagicMock() + + mock_client.get = AsyncMock(return_value=ref_response) + mock_client.post = AsyncMock(return_value=create_response) + + result = await github_client.create_branch("owner", "repo", "my-branch", base="develop") + + mock_client.get.assert_called_once_with("/repos/owner/repo/git/ref/heads/develop") + assert result["ref"] == "refs/heads/my-branch" + + +class TestCreateOrUpdateFile: + @pytest.mark.asyncio + async def test_creates_new_file(self, github_client): + mock_client = github_client._client + + response = MagicMock() + response.json.return_value = { + "content": {"sha": "newsha123", "path": "proposals/TEST-123-my-feature.md"} + } + response.raise_for_status = MagicMock() + mock_client.put = AsyncMock(return_value=response) + + result = await github_client.create_or_update_file( + owner="owner", + repo="repo", + path="proposals/TEST-123-my-feature.md", + content="# PRD content", + message="Add PRD for TEST-123", + branch="forge/prd/test-123", + ) + + call_args = mock_client.put.call_args + assert call_args[0][0] == "/repos/owner/repo/contents/proposals/TEST-123-my-feature.md" + body = call_args[1]["json"] + assert body["branch"] == "forge/prd/test-123" + assert body["message"] == "Add PRD for TEST-123" + assert base64.b64decode(body["content"]).decode() == "# PRD content" + assert "sha" not in body + + @pytest.mark.asyncio + async def test_updates_existing_file_with_sha(self, github_client): + mock_client = github_client._client + + response = MagicMock() + response.json.return_value = { + "content": {"sha": "updatedsha", "path": "proposals/TEST-123-my-feature.md"} + } + response.raise_for_status = MagicMock() + mock_client.put = AsyncMock(return_value=response) + + await github_client.create_or_update_file( + owner="owner", + repo="repo", + path="proposals/TEST-123-my-feature.md", + content="# Updated PRD", + message="Update PRD for TEST-123", + branch="forge/prd/test-123", + sha="oldsha456", + ) + + body = mock_client.put.call_args[1]["json"] + assert body["sha"] == "oldsha456" + + +class TestGetFileContents: + @pytest.mark.asyncio + async def test_returns_file_metadata(self, github_client): + mock_client = github_client._client + + response = MagicMock() + response.status_code = 200 + response.json.return_value = { + "sha": "filesha789", + "path": "proposals/TEST-123-my-feature.md", + "content": base64.b64encode(b"# PRD").decode(), + } + response.raise_for_status = MagicMock() + mock_client.get = AsyncMock(return_value=response) + + result = await github_client.get_file_contents( + "owner", "repo", "proposals/TEST-123-my-feature.md", "forge/prd/test-123" + ) + + assert result["sha"] == "filesha789" + mock_client.get.assert_called_once_with( + "/repos/owner/repo/contents/proposals/TEST-123-my-feature.md", + params={"ref": "forge/prd/test-123"}, + ) + + @pytest.mark.asyncio + async def test_returns_none_on_404(self, github_client): + mock_client = github_client._client + + response = MagicMock() + response.status_code = 404 + response.raise_for_status = MagicMock( + side_effect=httpx.HTTPStatusError( + "Not Found", request=MagicMock(), response=response + ) + ) + mock_client.get = AsyncMock(return_value=response) + + result = await github_client.get_file_contents( + "owner", "repo", "proposals/nonexistent.md", "main" + ) + + assert result is None diff --git a/tests/unit/orchestrator/nodes/test_generate_prd.py b/tests/unit/orchestrator/nodes/test_generate_prd.py index c0990a45..8dc012ac 100644 --- a/tests/unit/orchestrator/nodes/test_generate_prd.py +++ b/tests/unit/orchestrator/nodes/test_generate_prd.py @@ -41,6 +41,7 @@ def mock_jira(self): mock.update_description = AsyncMock() mock.set_workflow_label = AsyncMock() mock.add_structured_comment = AsyncMock() + mock.get_prd_proposals_repo = AsyncMock(return_value=None) mock.close = AsyncMock() return mock @@ -154,6 +155,7 @@ def mock_jira(self): mock.update_description = AsyncMock() mock.add_structured_comment = AsyncMock() mock.add_comment = AsyncMock() + mock.get_prd_proposals_repo = AsyncMock(return_value=None) mock.close = AsyncMock() return mock diff --git a/tests/unit/orchestrator/test_worker_prd_pr.py b/tests/unit/orchestrator/test_worker_prd_pr.py new file mode 100644 index 00000000..92a371a9 --- /dev/null +++ b/tests/unit/orchestrator/test_worker_prd_pr.py @@ -0,0 +1,304 @@ +"""Tests for PRD PR event handling in the worker.""" + +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from forge.models.events import EventSource +from forge.orchestrator.worker import OrchestratorWorker +from forge.queue.models import QueueMessage + + +def _make_message(event_type: str, payload: dict, ticket_key: str = "TEST-123") -> QueueMessage: + return QueueMessage( + message_id="msg-1", + event_id="evt-1", + source=EventSource.GITHUB, + event_type=event_type, + ticket_key=ticket_key, + payload=payload, + ) + + +def _prd_gate_state(**overrides) -> dict: + base = { + "ticket_key": "TEST-123", + "current_node": "prd_approval_gate", + "is_paused": True, + "prd_pr_number": 7, + "prd_pr_repo": "org/proposals", + "prd_pr_branch": "forge/prd/test-123", + "prd_pr_url": "https://github.com/org/proposals/pull/7", + "context": {}, + "last_error": None, + "revision_requested": False, + "feedback_comment": None, + "is_question": False, + "retry_count": 0, + "is_blocked": False, + } + base.update(overrides) + return base + + +@pytest.fixture +def worker(): + with patch("forge.orchestrator.worker.get_checkpointer"): + w = OrchestratorWorker.__new__(OrchestratorWorker) + w._post_terminal_error_comment = AsyncMock() + return w + + +class TestIsPrdPrEvent: + def test_true_for_matching_repo_and_pr(self, worker): + msg = _make_message("pull_request_review:submitted", { + "repository": {"full_name": "org/proposals"}, + "pull_request": {"number": 7}, + }) + state = _prd_gate_state() + assert worker._is_prd_pr_event(msg, state) is True + + def test_false_for_wrong_repo(self, worker): + msg = _make_message("pull_request_review:submitted", { + "repository": {"full_name": "org/other-repo"}, + "pull_request": {"number": 7}, + }) + state = _prd_gate_state() + assert worker._is_prd_pr_event(msg, state) is False + + def test_false_for_wrong_pr_number(self, worker): + msg = _make_message("pull_request_review:submitted", { + "repository": {"full_name": "org/proposals"}, + "pull_request": {"number": 99}, + }) + state = _prd_gate_state() + assert worker._is_prd_pr_event(msg, state) is False + + def test_false_when_no_prd_pr_in_state(self, worker): + msg = _make_message("pull_request_review:submitted", { + "repository": {"full_name": "org/proposals"}, + "pull_request": {"number": 7}, + }) + state = _prd_gate_state(prd_pr_number=None, prd_pr_repo=None) + assert worker._is_prd_pr_event(msg, state) is False + + def test_false_for_jira_events(self, worker): + msg = QueueMessage( + message_id="msg-1", + event_id="evt-1", + source=EventSource.JIRA, + event_type="issue_updated", + ticket_key="TEST-123", + payload={}, + ) + state = _prd_gate_state() + assert worker._is_prd_pr_event(msg, state) is False + + def test_matches_issue_comment_with_issue_number(self, worker): + msg = _make_message("issue_comment:created", { + "repository": {"full_name": "org/proposals"}, + "issue": {"number": 7}, + }) + state = _prd_gate_state() + assert worker._is_prd_pr_event(msg, state) is True + + +class TestHandlePrdPrMerge: + @pytest.mark.asyncio + async def test_pr_merge_sets_approved(self, worker): + msg = _make_message("pull_request:closed", { + "repository": {"full_name": "org/proposals"}, + "pull_request": {"number": 7, "merged": True}, + }) + state = _prd_gate_state() + + with patch("forge.orchestrator.worker.JiraClient") as MockJira: + mock_jira = MagicMock() + mock_jira.set_workflow_label = AsyncMock() + mock_jira.close = AsyncMock() + MockJira.return_value = mock_jira + + result = await worker._handle_resume_event(msg, state) + + assert result["is_paused"] is False + mock_jira.set_workflow_label.assert_called_once() + + @pytest.mark.asyncio + async def test_pr_close_without_merge_is_ignored(self, worker): + msg = _make_message("pull_request:closed", { + "repository": {"full_name": "org/proposals"}, + "pull_request": {"number": 7, "merged": False}, + }) + state = _prd_gate_state() + + result = await worker._handle_resume_event(msg, state) + + # Should remain paused -- closed without merge is not approval + assert result.get("is_paused", True) is True + + +class TestHandlePrdPrReview: + @pytest.mark.asyncio + async def test_changes_requested_sets_feedback(self, worker): + msg = _make_message("pull_request_review:submitted", { + "repository": {"full_name": "org/proposals"}, + "pull_request": {"number": 7}, + "review": {"state": "changes_requested", "body": "Please add more detail"}, + }) + state = _prd_gate_state() + + with patch("forge.orchestrator.worker.GitHubClient") as MockGH: + mock_gh = MagicMock() + mock_gh.get_pull_request_review_comments = AsyncMock(return_value=[]) + mock_gh.close = AsyncMock() + MockGH.return_value = mock_gh + + result = await worker._handle_resume_event(msg, state) + + assert result["is_paused"] is False + assert result["revision_requested"] is True + assert "more detail" in result["feedback_comment"] + + @pytest.mark.asyncio + async def test_approved_review_is_ignored(self, worker): + msg = _make_message("pull_request_review:submitted", { + "repository": {"full_name": "org/proposals"}, + "pull_request": {"number": 7}, + "review": {"state": "approved", "body": "LGTM"}, + }) + state = _prd_gate_state() + + result = await worker._handle_resume_event(msg, state) + + # Should remain paused -- review approval is not an approval signal + assert result.get("is_paused", True) is True + + +class TestHandlePrdPrComment: + @pytest.mark.asyncio + async def test_comment_sets_feedback(self, worker): + msg = _make_message("issue_comment:created", { + "repository": {"full_name": "org/proposals"}, + "issue": {"number": 7}, + "comment": { + "body": "Please expand the scope section", + "user": {"login": "reviewer"}, + }, + "sender": {"login": "reviewer"}, + }) + state = _prd_gate_state() + + with patch("forge.orchestrator.worker.GitHubClient") as MockGH: + mock_gh = MagicMock() + mock_gh.get_authenticated_user = AsyncMock(return_value={"login": "forge-bot"}) + mock_gh.close = AsyncMock() + MockGH.return_value = mock_gh + + result = await worker._handle_resume_event(msg, state) + + assert result["is_paused"] is False + assert result["revision_requested"] is True + assert "scope section" in result["feedback_comment"] + + @pytest.mark.asyncio + async def test_self_comment_is_ignored(self, worker): + msg = _make_message("issue_comment:created", { + "repository": {"full_name": "org/proposals"}, + "issue": {"number": 7}, + "comment": { + "body": "PRD has been revised based on feedback.", + "user": {"login": "forge-bot"}, + }, + "sender": {"login": "forge-bot"}, + }) + state = _prd_gate_state() + + with patch("forge.orchestrator.worker.GitHubClient") as MockGH: + mock_gh = MagicMock() + mock_gh.get_authenticated_user = AsyncMock(return_value={"login": "forge-bot"}) + mock_gh.close = AsyncMock() + MockGH.return_value = mock_gh + + result = await worker._handle_resume_event(msg, state) + + # Should remain paused -- self-comment ignored + assert result.get("is_paused", True) is True + + @pytest.mark.asyncio + async def test_question_comment_sets_question_flag(self, worker): + msg = _make_message("issue_comment:created", { + "repository": {"full_name": "org/proposals"}, + "issue": {"number": 7}, + "comment": { + "body": "?Why did you choose REST over GraphQL?", + "user": {"login": "reviewer"}, + }, + "sender": {"login": "reviewer"}, + }) + state = _prd_gate_state() + + with patch("forge.orchestrator.worker.GitHubClient") as MockGH: + mock_gh = MagicMock() + mock_gh.get_authenticated_user = AsyncMock(return_value={"login": "forge-bot"}) + mock_gh.close = AsyncMock() + MockGH.return_value = mock_gh + + result = await worker._handle_resume_event(msg, state) + + assert result["is_paused"] is False + assert result.get("is_question") is True + assert "REST" in result["feedback_comment"] + + +class TestJiraCommentIgnoredInPrMode: + @pytest.mark.asyncio + async def test_jira_comment_ignored_when_prd_pr_exists(self, worker): + """Jira comments should not trigger feedback when PRD review is on GitHub PR.""" + msg = QueueMessage( + message_id="msg-jira-1", + event_id="evt-jira-1", + source=EventSource.JIRA, + event_type="issue_comment_created", + ticket_key="TEST-123", + payload={ + "comment": { + "body": "This is a Jira comment that should be ignored", + }, + "changelog": {"items": []}, + "issue": {"fields": {"labels": ["forge:managed", "forge:prd-pending"]}}, + }, + ) + state = _prd_gate_state() + + result = await worker._handle_resume_event(msg, state) + + # Should remain paused — Jira comment ignored in PR mode + assert result.get("is_paused", True) is True + assert result.get("revision_requested") is not True + + @pytest.mark.asyncio + async def test_jira_comment_processed_when_no_prd_pr(self, worker): + """Jira comments should still work in normal Jira-only mode.""" + msg = QueueMessage( + message_id="msg-jira-2", + event_id="evt-jira-2", + source=EventSource.JIRA, + event_type="issue_comment_created", + ticket_key="TEST-123", + payload={ + "comment": { + "body": "Please expand the scope section", + }, + "changelog": {"items": []}, + "issue": {"fields": {"labels": ["forge:managed", "forge:prd-pending"]}}, + }, + ) + # No prd_pr_number — Jira-only mode + state = _prd_gate_state(prd_pr_number=None, prd_pr_repo=None) + + result = await worker._handle_resume_event(msg, state) + + # Should process the comment as feedback + assert result["is_paused"] is False + assert result["revision_requested"] is True + assert "scope section" in result["feedback_comment"] diff --git a/tests/unit/test_config_prd.py b/tests/unit/test_config_prd.py new file mode 100644 index 00000000..7149c6dd --- /dev/null +++ b/tests/unit/test_config_prd.py @@ -0,0 +1,36 @@ +"""Tests for PRD approval configuration settings.""" + +from forge.config import Settings + + +class TestPrdApprovalConfig: + def test_default_proposals_repo_is_empty(self): + settings = Settings( + jira_base_url="https://test.atlassian.net", + jira_api_token="test", + jira_user_email="test@example.com", + github_token="test", + anthropic_api_key="test", + ) + assert settings.prd_proposals_repo == "" + + def test_default_proposals_path(self): + settings = Settings( + jira_base_url="https://test.atlassian.net", + jira_api_token="test", + jira_user_email="test@example.com", + github_token="test", + anthropic_api_key="test", + ) + assert settings.prd_proposals_path == "proposals" + + def test_proposals_repo_can_be_set_as_global_fallback(self): + settings = Settings( + jira_base_url="https://test.atlassian.net", + jira_api_token="test", + jira_user_email="test@example.com", + github_token="test", + anthropic_api_key="test", + prd_proposals_repo="org/proposals", + ) + assert settings.prd_proposals_repo == "org/proposals" diff --git a/tests/unit/workflow/feature/test_prd_pr_state.py b/tests/unit/workflow/feature/test_prd_pr_state.py new file mode 100644 index 00000000..d1fd7cdb --- /dev/null +++ b/tests/unit/workflow/feature/test_prd_pr_state.py @@ -0,0 +1,43 @@ +"""Tests for PRD PR state fields.""" + +from forge.models.workflow import TicketType +from forge.workflow.feature.state import FeatureState, create_initial_feature_state + + +class TestPrdPrStateFields: + def test_initial_state_has_prd_pr_fields(self): + state = create_initial_feature_state( + ticket_key="TEST-123", + ticket_type=TicketType.FEATURE, + ) + assert state["prd_pr_url"] is None + assert state["prd_pr_number"] is None + assert state["prd_pr_repo"] is None + assert state["prd_pr_branch"] is None + assert state["prd_pr_file_path"] is None + + def test_prd_pr_fields_can_be_set(self): + state = create_initial_feature_state( + ticket_key="TEST-123", + ticket_type=TicketType.FEATURE, + prd_pr_url="https://github.com/org/proposals/pull/5", + prd_pr_number=5, + prd_pr_repo="org/proposals", + prd_pr_branch="forge/prd/test-123", + prd_pr_file_path="proposals/TEST-123-my-feature.md", + ) + assert state["prd_pr_url"] == "https://github.com/org/proposals/pull/5" + assert state["prd_pr_number"] == 5 + assert state["prd_pr_repo"] == "org/proposals" + assert state["prd_pr_branch"] == "forge/prd/test-123" + assert state["prd_pr_file_path"] == "proposals/TEST-123-my-feature.md" + + def test_prd_pr_fields_separate_from_implementation_pr(self): + state = create_initial_feature_state( + ticket_key="TEST-123", + ticket_type=TicketType.FEATURE, + ) + assert state["current_pr_url"] is None + assert state["current_pr_number"] is None + assert state["prd_pr_url"] is None + assert state["prd_pr_number"] is None diff --git a/tests/unit/workflow/nodes/test_generation_context.py b/tests/unit/workflow/nodes/test_generation_context.py index 794de03b..3aac3f7a 100644 --- a/tests/unit/workflow/nodes/test_generation_context.py +++ b/tests/unit/workflow/nodes/test_generation_context.py @@ -16,6 +16,7 @@ def create_mock_jira_client(): mock.update_description = AsyncMock() mock.add_structured_comment = AsyncMock() mock.set_workflow_label = AsyncMock() + mock.get_prd_proposals_repo = AsyncMock(return_value=None) return mock diff --git a/tests/unit/workflow/nodes/test_prd_pr.py b/tests/unit/workflow/nodes/test_prd_pr.py new file mode 100644 index 00000000..000db5aa --- /dev/null +++ b/tests/unit/workflow/nodes/test_prd_pr.py @@ -0,0 +1,131 @@ +"""Tests for PRD PR creation and update helpers.""" + +import re +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from forge.models.workflow import TicketType +from forge.workflow.feature.state import create_initial_feature_state + + +class TestSlugify: + def test_basic_slugify(self): + from forge.workflow.nodes.prd_generation import _slugify + + assert _slugify("My Feature Title") == "my-feature-title" + + def test_removes_special_chars(self): + from forge.workflow.nodes.prd_generation import _slugify + + assert _slugify("Define API for CaaS (v2)") == "define-api-for-caas-v2" + + def test_truncates_to_max_length(self): + from forge.workflow.nodes.prd_generation import _slugify + + long_title = "a" * 100 + result = _slugify(long_title, max_length=60) + assert len(result) <= 60 + + def test_strips_trailing_hyphens(self): + from forge.workflow.nodes.prd_generation import _slugify + + assert _slugify("trailing---") == "trailing" + + +class TestCreatePrdProposalPr: + @pytest.mark.asyncio + async def test_creates_branch_and_pr(self): + from forge.workflow.nodes.prd_generation import _create_prd_proposal_pr + + mock_gh = MagicMock() + mock_gh.create_branch = AsyncMock(return_value={"ref": "refs/heads/forge/prd/test-123"}) + mock_gh.create_or_update_file = AsyncMock( + return_value={"content": {"sha": "filesha"}} + ) + mock_gh.create_pull_request = AsyncMock( + return_value={ + "number": 7, + "html_url": "https://github.com/org/proposals/pull/7", + } + ) + mock_gh.close = AsyncMock() + + mock_jira = MagicMock() + mock_jira.add_comment = AsyncMock() + mock_jira.set_workflow_label = AsyncMock() + mock_jira.close = AsyncMock() + + mock_settings = MagicMock() + mock_settings.prd_proposals_repo = "org/proposals" + mock_settings.prd_proposals_path = "proposals" + + with ( + patch("forge.workflow.nodes.prd_generation.GitHubClient", return_value=mock_gh), + patch("forge.workflow.nodes.prd_generation.JiraClient", return_value=mock_jira), + patch("forge.workflow.nodes.prd_generation.get_settings", return_value=mock_settings), + patch( + "forge.workflow.nodes.prd_generation.set_pr_ticket_index", + new_callable=AsyncMock, + ) as mock_index, + ): + result = await _create_prd_proposal_pr( + ticket_key="TEST-123", + prd_content="# My PRD", + summary="My Feature", + proposals_repo="org/proposals", + ) + + assert result["prd_pr_number"] == 7 + assert result["prd_pr_url"] == "https://github.com/org/proposals/pull/7" + assert result["prd_pr_repo"] == "org/proposals" + assert result["prd_pr_branch"] == "forge/prd/test-123" + assert result["prd_pr_file_path"] == "proposals/TEST-123-my-feature.md" + + mock_gh.create_branch.assert_called_once_with("org", "proposals", "forge/prd/test-123") + mock_gh.create_pull_request.assert_called_once() + mock_jira.add_comment.assert_called_once() + mock_jira.set_workflow_label.assert_called_once() + mock_index.assert_called_once() + + +class TestUpdatePrdProposalPr: + @pytest.mark.asyncio + async def test_updates_file_on_branch(self): + from forge.workflow.nodes.prd_generation import _update_prd_proposal_pr + + mock_gh = MagicMock() + mock_gh.get_file_contents = AsyncMock( + return_value={"sha": "oldsha", "path": "proposals/TEST-123-my-feature.md"} + ) + mock_gh.create_or_update_file = AsyncMock( + return_value={"content": {"sha": "newsha"}} + ) + mock_gh.create_issue_comment = AsyncMock() + mock_gh.close = AsyncMock() + + state = create_initial_feature_state( + ticket_key="TEST-123", + ticket_type=TicketType.FEATURE, + prd_pr_branch="forge/prd/test-123", + prd_pr_repo="org/proposals", + prd_pr_number=7, + prd_pr_url="https://github.com/org/proposals/pull/7", + prd_pr_file_path="proposals/TEST-123-my-feature.md", + ) + + with patch("forge.workflow.nodes.prd_generation.GitHubClient", return_value=mock_gh): + await _update_prd_proposal_pr( + ticket_key="TEST-123", + prd_content="# Revised PRD", + state=state, + ) + + mock_gh.get_file_contents.assert_called_once_with( + "org", "proposals", "proposals/TEST-123-my-feature.md", "forge/prd/test-123" + ) + mock_gh.create_or_update_file.assert_called_once() + call_kwargs = mock_gh.create_or_update_file.call_args[1] + assert call_kwargs["sha"] == "oldsha" + assert call_kwargs["path"] == "proposals/TEST-123-my-feature.md" + mock_gh.create_issue_comment.assert_called_once()