From 45f4537b557cee5617848cc9ded45b7752b86e45 Mon Sep 17 00:00:00 2001 From: Forge Date: Thu, 2 Jul 2026 12:40:26 +0000 Subject: [PATCH 1/2] [AISOS-2083] Execute task takeover changes for AISOS-2083 Auto-committed by Forge container fallback. --- src/forge/cli.py | 45 +++++++++++++++---- src/forge/integrations/github/client.py | 18 +++++--- src/forge/integrations/jira/client.py | 59 ++++++++++++++++++++++--- src/forge/workflow/nodes/pr_creation.py | 4 ++ 4 files changed, 106 insertions(+), 20 deletions(-) diff --git a/src/forge/cli.py b/src/forge/cli.py index fcd0f109..36016aac 100644 --- a/src/forge/cli.py +++ b/src/forge/cli.py @@ -502,15 +502,42 @@ async def cmd_project_setup(args: argparse.Namespace) -> int: try: # forge.repos if args.repo: - invalid = [r for r in args.repo if "/" not in r] - if invalid: - print( - f"Error: invalid repo format (expected owner/repo): {invalid}", - file=sys.stderr, - ) - return 1 - await jira.set_project_property(project_key, "forge.repos", args.repo) - print(f"[OK] forge.repos = {args.repo}") + parsed_repos = [] + for r in args.repo: + if r.startswith("{"): + try: + repo_dict = json.loads(r) + except Exception as e: + print( + f"Error: failed to parse JSON repo config {r!r}: {e}", + file=sys.stderr, + ) + return 1 + if not isinstance(repo_dict, dict) or "name" not in repo_dict: + print( + f"Error: JSON repo config must be a dictionary with a 'name' key, got: {r!r}", + file=sys.stderr, + ) + return 1 + name = repo_dict["name"] + if not isinstance(name, str) or "/" not in name: + print( + f"Error: repo name in JSON config must contain '/', got: {name!r}", + file=sys.stderr, + ) + return 1 + parsed_repos.append(repo_dict) + else: + if "/" not in r: + print( + f"Error: invalid repo format (expected owner/repo): {r!r}", + file=sys.stderr, + ) + return 1 + parsed_repos.append(r) + + await jira.set_project_property(project_key, "forge.repos", parsed_repos) + print(f"[OK] forge.repos = {parsed_repos}") # forge.default_repo if args.default_repo: diff --git a/src/forge/integrations/github/client.py b/src/forge/integrations/github/client.py index baaa7108..5ac8b9bd 100644 --- a/src/forge/integrations/github/client.py +++ b/src/forge/integrations/github/client.py @@ -80,6 +80,7 @@ async def create_pull_request( body: str, head: str, base: str = "main", + draft: bool = False, ) -> dict[str, Any]: """Create a new pull request. @@ -90,19 +91,24 @@ async def create_pull_request( body: PR description. head: Source branch name. base: Target branch name. + draft: Whether the PR should be created as a draft. Returns: API response with PR details. """ client = await self._get_client() + payload = { + "title": title, + "body": body, + "head": head, + "base": base, + } + if draft: + payload["draft"] = True + response = await client.post( f"/repos/{owner}/{repo}/pulls", - json={ - "title": title, - "body": body, - "head": head, - "base": base, - }, + json=payload, ) response.raise_for_status() data = response.json() diff --git a/src/forge/integrations/jira/client.py b/src/forge/integrations/jira/client.py index 46abecb4..fc88c508 100644 --- a/src/forge/integrations/jira/client.py +++ b/src/forge/integrations/jira/client.py @@ -947,14 +947,63 @@ async def get_project_repos(self, project_key: str) -> list[str]: value = await self.get_project_property(project_key, "forge.repos") if value is None: raise MissingProjectConfig(f"forge.repos not set for project {project_key}") - if not isinstance(value, list) or any( - not isinstance(r, str) or "/" not in r for r in value - ): + if not isinstance(value, list): raise MissingProjectConfig( f"forge.repos for project {project_key} is malformed: {value!r}" ) - logger.info(f"Project {project_key}: repos from Jira property: {value}") - return value + + repos = [] + for r in value: + if isinstance(r, str): + if "/" not in r: + raise MissingProjectConfig( + f"forge.repos for project {project_key} is malformed: {value!r}" + ) + repos.append(r) + elif isinstance(r, dict): + name = r.get("name") + if not isinstance(name, str) or "/" not in name: + raise MissingProjectConfig( + f"forge.repos for project {project_key} is malformed: {value!r}" + ) + repos.append(name) + else: + raise MissingProjectConfig( + f"forge.repos for project {project_key} is malformed: {value!r}" + ) + + logger.info(f"Project {project_key}: repos from Jira property: {repos}") + return repos + + async def is_repo_draft(self, project_key: str, repo_name: str) -> bool: + """Check if draft PRs are enabled for a given repository. + + Args: + project_key: The Jira project key. + repo_name: Name of the repository (e.g. "owner/repo"). + + Returns: + True if draft is enabled, False otherwise. + """ + try: + value = await self.get_project_property(project_key, "forge.repos") + except Exception: + return False + + if not isinstance(value, list): + return False + + for r in value: + if isinstance(r, dict): + name = r.get("name") + if isinstance(name, str) and name.lower() == repo_name.lower(): + # Check "draft" or "draft_pr" + draft = r.get("draft") + if draft is None: + draft = r.get("draft_pr") + return bool(draft) + + return False async def get_project_default_repo(self, project_key: str) -> str: """Fetch the forge.default_repo project property. diff --git a/src/forge/workflow/nodes/pr_creation.py b/src/forge/workflow/nodes/pr_creation.py index 225bed8a..5f3c8154 100644 --- a/src/forge/workflow/nodes/pr_creation.py +++ b/src/forge/workflow/nodes/pr_creation.py @@ -185,6 +185,9 @@ async def create_pull_request(state: WorkflowState) -> WorkflowState: # Create PR from fork to upstream # Head format: "fork_owner:branch_name" + project_key = ticket_key.split("-")[0] if "-" in ticket_key else ticket_key + is_draft = await jira.is_repo_draft(project_key, current_repo) + pr_data = await github.create_pull_request( owner=owner, repo=repo, @@ -192,6 +195,7 @@ async def create_pull_request(state: WorkflowState) -> WorkflowState: body=pr_body, head=f"{fork_owner}:{branch_name}", base="main", + draft=is_draft, ) pr_url = pr_data.get("html_url", "") From 7ef7c2b6fe5298f6895ac0746940ea9999614e3d Mon Sep 17 00:00:00 2001 From: Forge Date: Thu, 2 Jul 2026 12:48:36 +0000 Subject: [PATCH 2/2] [AISOS-2083] Add unit tests and configuration documentation for draft pull requests Detailed description: - Added comprehensive unit tests in tests/unit/workflow/nodes/test_pr_creation_draft.py to verify draft and non-draft PR creation behavior across JiraClient, GitHubClient, and the PR creation node. - Updated mock configurations in existing test suites (test_code_review.py and test_pr_creation_pr_number.py) to include mock implementations of is_repo_draft. - Expanded docs/reference/config.md to document the structured repository configuration object schema, illustrating how to enable draft PRs under the 'forge.repos' project property. Closes: AISOS-2083 --- docs/reference/config.md | 13 + tests/unit/workflow/nodes/test_code_review.py | 1 + .../workflow/nodes/test_pr_creation_draft.py | 323 ++++++++++++++++++ .../nodes/test_pr_creation_pr_number.py | 1 + 4 files changed, 338 insertions(+) create mode 100644 tests/unit/workflow/nodes/test_pr_creation_draft.py diff --git a/docs/reference/config.md b/docs/reference/config.md index 72f94b5d..1f8bbecb 100644 --- a/docs/reference/config.md +++ b/docs/reference/config.md @@ -60,6 +60,19 @@ curl -X PUT \ -u "you@example.com:YOUR_API_TOKEN" \ -d '["org/repo1", "org/repo2"]' +# Alternatively, configure a repository with additional metadata (like enabling draft PRs) using an object: +curl -X PUT \ + "https://your-org.atlassian.net/rest/api/3/project/MYPROJ/properties/forge.repos" \ + -H "Content-Type: application/json" \ + -u "you@example.com:YOUR_API_TOKEN" \ + -d '[ + "org/repo1", + { + "name": "org/repo2", + "draft": true + } + ]' + # Default repo when no explicit assignment is made curl -X PUT \ "https://your-org.atlassian.net/rest/api/3/project/MYPROJ/properties/forge.default_repo" \ diff --git a/tests/unit/workflow/nodes/test_code_review.py b/tests/unit/workflow/nodes/test_code_review.py index a08f2bef..85c4c1ba 100644 --- a/tests/unit/workflow/nodes/test_code_review.py +++ b/tests/unit/workflow/nodes/test_code_review.py @@ -281,6 +281,7 @@ async def test_sync_called_after_pr_creation(self): mock_jira.get_issue = AsyncMock(return_value=MagicMock(summary="Test feature")) mock_jira.add_comment = AsyncMock() mock_jira.create_remote_link = AsyncMock() + mock_jira.is_repo_draft = AsyncMock(return_value=False) mock_jira.close = AsyncMock() mock_git = MagicMock() diff --git a/tests/unit/workflow/nodes/test_pr_creation_draft.py b/tests/unit/workflow/nodes/test_pr_creation_draft.py new file mode 100644 index 00000000..85268445 --- /dev/null +++ b/tests/unit/workflow/nodes/test_pr_creation_draft.py @@ -0,0 +1,323 @@ +"""Unit tests for draft PR creation behavior and repository metadata configuration.""" + +from pathlib import Path +from unittest.mock import ANY, AsyncMock, MagicMock, patch + +import pytest + +from forge.integrations.github.client import GitHubClient +from forge.integrations.jira.client import JiraClient, MissingProjectConfig +from forge.workflow.feature.state import create_initial_feature_state +from forge.workflow.nodes.pr_creation import create_pull_request + + +def create_mock_github_client(pr_number=123, pr_url="https://github.com/owner/repo/pull/123"): + """Create a mock GitHubClient with configurable PR data.""" + mock = MagicMock() + mock.close = AsyncMock() + mock.get_or_create_fork = AsyncMock( + return_value={ + "owner": {"login": "fork-owner"}, + "name": "repo", + } + ) + mock.sync_fork_with_upstream = AsyncMock() + + pr_data = { + "html_url": pr_url, + } + if pr_number is not None: + pr_data["number"] = pr_number + + mock.create_pull_request = AsyncMock(return_value=pr_data) + return mock + + +def create_mock_jira_client(): + """Create a mock JiraClient.""" + mock = MagicMock() + mock.close = AsyncMock() + mock.add_comment = AsyncMock() + mock.create_remote_link = AsyncMock() + mock.get_issue = AsyncMock() + mock.set_workflow_label = AsyncMock() + mock.is_repo_draft = AsyncMock(return_value=False) + + # Mock issue with summary + mock_issue = MagicMock() + mock_issue.summary = "Test feature" + mock.get_issue.return_value = mock_issue + + return mock + + +def create_mock_git_operations(): + """Create a mock GitOperations.""" + mock = MagicMock() + mock.add_fork_remote = MagicMock() + mock.push_to_fork = MagicMock() + + mock_result = MagicMock() + mock_result.stdout = "abc123 Test commit\n\nTest commit body" + mock._run_git = MagicMock(return_value=mock_result) + + return mock + + +def create_mock_workspace(): + """Create a mock Workspace.""" + mock = MagicMock() + mock.path = Path("/tmp/test-workspace") + return mock + + +class TestJiraConfigParser: + """Tests JiraClient configuration parsing for repos metadata and draft PR settings.""" + + @pytest.fixture + def mock_jira_client(self): + """Create a JiraClient with mocked settings.""" + with patch("forge.integrations.jira.client.get_settings") as mock_settings: + mock_settings.return_value.jira_base_url = "https://test.atlassian.net" + mock_settings.return_value.jira_api_token = MagicMock() + mock_settings.return_value.jira_api_token.get_secret_value.return_value = "token" + mock_settings.return_value.jira_user_email = "test@example.com" + return JiraClient() + + @pytest.mark.asyncio + async def test_get_project_repos_mixed_list(self, mock_jira_client): + """Successfully parses mixed list of strings and metadata dicts in forge.repos.""" + mock_jira_client.get_project_property = AsyncMock( + return_value=[ + "owner/repo1", + {"name": "owner/repo2", "draft": True}, + {"name": "owner/repo3", "draft_pr": True}, + {"name": "owner/repo4"}, + ] + ) + + repos = await mock_jira_client.get_project_repos("PROJ") + assert repos == ["owner/repo1", "owner/repo2", "owner/repo3", "owner/repo4"] + + @pytest.mark.asyncio + async def test_get_project_repos_malformed_raises_error(self, mock_jira_client): + """Raises MissingProjectConfig if forge.repos is not a list or has malformed items.""" + # Malformed: not a list + mock_jira_client.get_project_property = AsyncMock(return_value="not-a-list") + with pytest.raises(MissingProjectConfig): + await mock_jira_client.get_project_repos("PROJ") + + # Malformed: string without '/' + mock_jira_client.get_project_property = AsyncMock(return_value=["malformed-name"]) + with pytest.raises(MissingProjectConfig): + await mock_jira_client.get_project_repos("PROJ") + + # Malformed: dict without name + mock_jira_client.get_project_property = AsyncMock(return_value=[{"draft": True}]) + with pytest.raises(MissingProjectConfig): + await mock_jira_client.get_project_repos("PROJ") + + @pytest.mark.asyncio + async def test_is_repo_draft(self, mock_jira_client): + """Correctly resolves draft setting for various repo configurations.""" + mock_jira_client.get_project_property = AsyncMock( + return_value=[ + "owner/repo1", + {"name": "owner/repo2", "draft": True}, + {"name": "owner/repo3", "draft_pr": True}, + {"name": "owner/repo4", "draft": False}, + {"name": "owner/repo5"}, + ] + ) + + # String entry: should be False + assert await mock_jira_client.is_repo_draft("PROJ", "owner/repo1") is False + + # Dict with draft=True: should be True + assert await mock_jira_client.is_repo_draft("PROJ", "owner/repo2") is True + + # Dict with draft_pr=True: should be True + assert await mock_jira_client.is_repo_draft("PROJ", "owner/repo3") is True + + # Case-insensitive resolution: should be True + assert await mock_jira_client.is_repo_draft("PROJ", "OwNeR/RePo2") is True + + # Dict with draft=False: should be False + assert await mock_jira_client.is_repo_draft("PROJ", "owner/repo4") is False + + # Dict without draft keys: should be False + assert await mock_jira_client.is_repo_draft("PROJ", "owner/repo5") is False + + # Non-existent repository: should be False + assert await mock_jira_client.is_repo_draft("PROJ", "owner/nonexistent") is False + + @pytest.mark.asyncio + async def test_is_repo_draft_on_missing_property(self, mock_jira_client): + """is_repo_draft should gracefully fall back to False if project property is missing or errors.""" + mock_jira_client.get_project_property = AsyncMock(side_effect=Exception("API Error")) + assert await mock_jira_client.is_repo_draft("PROJ", "owner/repo1") is False + + +class TestGitHubClientDraft: + """Tests GitHubClient PR creation request payloads with draft options.""" + + @pytest.fixture + def mock_github_client(self): + """Create a GitHubClient with mocked settings.""" + with patch("forge.integrations.github.client.get_settings") as mock_settings: + mock_settings.return_value.github_token = MagicMock() + mock_settings.return_value.github_token.get_secret_value.return_value = "token" + mock_settings.return_value.github_fork_owner = "" + return GitHubClient() + + @pytest.mark.asyncio + async def test_create_pull_request_with_draft_true(self, mock_github_client): + """create_pull_request passes draft=True in payload when draft is True.""" + mock_response = MagicMock() + mock_response.status_code = 201 + mock_response.json.return_value = { + "number": 123, + "html_url": "https://github.com/owner/repo/pull/123", + } + mock_response.raise_for_status = MagicMock() + + with patch.object(mock_github_client, "_get_client") as mock_get_client: + mock_http_client = AsyncMock() + mock_http_client.post = AsyncMock(return_value=mock_response) + mock_get_client.return_value = mock_http_client + + await mock_github_client.create_pull_request( + owner="owner", + repo="repo", + title="test pr", + body="body", + head="branch", + base="main", + draft=True, + ) + + mock_http_client.post.assert_called_once() + call_json = mock_http_client.post.call_args.kwargs["json"] + assert call_json["draft"] is True + + @pytest.mark.asyncio + async def test_create_pull_request_with_draft_false(self, mock_github_client): + """create_pull_request does not include draft in payload when draft is False.""" + mock_response = MagicMock() + mock_response.status_code = 201 + mock_response.json.return_value = { + "number": 123, + "html_url": "https://github.com/owner/repo/pull/123", + } + mock_response.raise_for_status = MagicMock() + + with patch.object(mock_github_client, "_get_client") as mock_get_client: + mock_http_client = AsyncMock() + mock_http_client.post = AsyncMock(return_value=mock_response) + mock_get_client.return_value = mock_http_client + + await mock_github_client.create_pull_request( + owner="owner", + repo="repo", + title="test pr", + body="body", + head="branch", + base="main", + draft=False, + ) + + mock_http_client.post.assert_called_once() + call_json = mock_http_client.post.call_args.kwargs["json"] + assert "draft" not in call_json + + +class TestPRCreationNodeDraft: + """Tests the PR creation node workflow when draft PR option is enabled vs disabled.""" + + @pytest.mark.asyncio + async def test_create_pr_node_invokes_github_with_draft_true(self): + """Workflow node should invoke GitHubClient with draft=True if repository configured as draft.""" + mock_github = create_mock_github_client(pr_number=101) + mock_jira = create_mock_jira_client() + mock_jira.is_repo_draft = AsyncMock(return_value=True) + mock_git = create_mock_git_operations() + + state = create_initial_feature_state( + ticket_key="FEAT-123", + current_repo="owner/repo", + ) + state["workspace_path"] = "/tmp/test-workspace" + state["implemented_tasks"] = ["TASK-1"] + state["context"] = {"branch_name": "feat/test"} + + with ( + patch("forge.workflow.nodes.pr_creation.GitHubClient", return_value=mock_github), + patch("forge.workflow.nodes.pr_creation.JiraClient", return_value=mock_jira), + patch("forge.workflow.nodes.pr_creation.GitOperations", return_value=mock_git), + patch( + "forge.workflow.nodes.pr_creation.Workspace", return_value=create_mock_workspace() + ), + patch( + "forge.workflow.nodes.pr_creation.check_merge_conflicts", return_value=(False, []) + ), + patch("forge.workflow.nodes.pr_creation.sync_pr_description", new_callable=AsyncMock), + ): + await create_pull_request(state) + + # Verify is_repo_draft was resolved + mock_jira.is_repo_draft.assert_called_once_with("FEAT", "owner/repo") + + # Verify create_pull_request was called with draft=True + mock_github.create_pull_request.assert_called_once_with( + owner="owner", + repo="repo", + title="[FEAT-123] Test feature", + body=ANY, + head="fork-owner:feat/test", + base="main", + draft=True, + ) + + @pytest.mark.asyncio + async def test_create_pr_node_invokes_github_with_draft_false(self): + """Workflow node should invoke GitHubClient with draft=False if repository is not configured as draft.""" + mock_github = create_mock_github_client(pr_number=102) + mock_jira = create_mock_jira_client() + mock_jira.is_repo_draft = AsyncMock(return_value=False) + mock_git = create_mock_git_operations() + + state = create_initial_feature_state( + ticket_key="FEAT-123", + current_repo="owner/repo", + ) + state["workspace_path"] = "/tmp/test-workspace" + state["implemented_tasks"] = ["TASK-1"] + state["context"] = {"branch_name": "feat/test"} + + with ( + patch("forge.workflow.nodes.pr_creation.GitHubClient", return_value=mock_github), + patch("forge.workflow.nodes.pr_creation.JiraClient", return_value=mock_jira), + patch("forge.workflow.nodes.pr_creation.GitOperations", return_value=mock_git), + patch( + "forge.workflow.nodes.pr_creation.Workspace", return_value=create_mock_workspace() + ), + patch( + "forge.workflow.nodes.pr_creation.check_merge_conflicts", return_value=(False, []) + ), + patch("forge.workflow.nodes.pr_creation.sync_pr_description", new_callable=AsyncMock), + ): + await create_pull_request(state) + + # Verify is_repo_draft was resolved + mock_jira.is_repo_draft.assert_called_once_with("FEAT", "owner/repo") + + # Verify create_pull_request was called with draft=False + mock_github.create_pull_request.assert_called_once_with( + owner="owner", + repo="repo", + title="[FEAT-123] Test feature", + body=ANY, + head="fork-owner:feat/test", + base="main", + draft=False, + ) diff --git a/tests/unit/workflow/nodes/test_pr_creation_pr_number.py b/tests/unit/workflow/nodes/test_pr_creation_pr_number.py index b898b00a..5b0fd079 100644 --- a/tests/unit/workflow/nodes/test_pr_creation_pr_number.py +++ b/tests/unit/workflow/nodes/test_pr_creation_pr_number.py @@ -40,6 +40,7 @@ def create_mock_jira_client(): mock.create_remote_link = AsyncMock() mock.get_issue = AsyncMock() mock.set_workflow_label = AsyncMock() + mock.is_repo_draft = AsyncMock(return_value=False) # Mock issue with summary mock_issue = MagicMock()