diff --git a/CLAUDE.md b/CLAUDE.md index 257e6852c..e976693f6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -123,11 +123,16 @@ podman rm $(podman ps -a --filter name=forge- -q) ## Jira Comment Syntax -| Prefix | Effect | -|--------|--------| -| `!` | Revision request — triggers regeneration with feedback | +| Prefix / Command | Effect | +|------------------|--------| +| `!` | Revision request — triggers regeneration/revision with feedback | | `?` or `@forge ask` | Question — triggers Q&A answer | | `>option N` | RCA option selection (RCA Option Gate only) | +| `/forge approve` | Approve draft (Epic plan or Tasks) to provision tickets | +| `/forge remove ID` | Remove proposed draft item by ID and re-sequence | +| `/forge exclude ID` | Toggle exclusion of proposed draft item by ID | +| `/forge update ID key=val` | Update fields (`summary`, `description`, `repo`) of proposed draft item | +| `/forge add key=val` | Add a new proposed item to draft | | _(no prefix)_ | Informational — workflow ignores it | ## GitHub PR Comment Commands diff --git a/docs/developer-guide.md b/docs/developer-guide.md index c07d0d92a..917f670dd 100644 --- a/docs/developer-guide.md +++ b/docs/developer-guide.md @@ -873,6 +873,18 @@ curl -X POST http://localhost:8000/api/v1/webhooks/github \ | `/forge skip-gate ` | Skip named CI check | CI stages | | `/forge unskip-gate ` | Remove a skip | CI stages | +### Jira comment commands + +These commands are used on the parent Jira ticket during the draft review stages (Epic Plan and Tasks). + +| Command | Effect | Active at | +|---------|--------|-----------| +| `/forge approve` | Approve draft, provision sub-tickets, and delete draft attachment | `plan_approval_gate`, `task_approval_gate` | +| `/forge remove ` | Remove a draft item by local sequential ID | `plan_approval_gate`, `task_approval_gate` | +| `/forge exclude ` | Toggle the exclusion flag of a draft item | `plan_approval_gate`, `task_approval_gate` | +| `/forge update key=val` | Update fields (`summary`, `description`, `repo`) of a draft item | `plan_approval_gate`, `task_approval_gate` | +| `/forge add key=val` | Add a new proposed item to the draft | `plan_approval_gate`, `task_approval_gate` | + ### Jira labels | Label | Meaning | diff --git a/docs/guide/feature-workflow.md b/docs/guide/feature-workflow.md index 9fe57e441..a50b587c4 100644 --- a/docs/guide/feature-workflow.md +++ b/docs/guide/feature-workflow.md @@ -66,22 +66,31 @@ Forge generates a behavioral specification from the approved PRD, typically usin Forge breaks the feature into logical epics — high-level areas of work that map to implementation phases. -**Human action:** Review the epic plan. You have four options at this stage: +By default, Forge uses an interactive **Draft Review Flow** at this stage (unless YOLO mode is active): +1. Instead of creating Jira tickets immediately, Forge serializes the proposed epics into `forge-epics-draft.json` and uploads it as an attachment on the Feature ticket. +2. Forge posts a markdown table comment on the Feature ticket outlining the proposed Epics. +3. The workflow pauses at `plan_approval_gate`. -| Action | How | -|--------|-----| -| Approve | Change label to `forge:plan-approved` | -| Ask a question | Comment with `?` prefix — Forge answers without re-decomposing | -| Revise one epic | `!` comment on the **specific epic sub-ticket** — Forge updates only that epic | -| Redo the full decomposition | `!` comment on the **feature ticket** — Forge regenerates all epics with your feedback | +**Human action:** Review the epic plan draft. You have several options at this stage: + +| Action | How | Description | +|--------|-----|-------------| +| **Approve** | Comment `/forge approve` OR set label to `forge:plan-approved` | Forge provisions the Epic sub-tickets on Jira from the draft, deletes the draft attachment, and advances to Task Generation. | +| **Direct Edit** | Use `/forge` commands (e.g. `/forge update`, `/forge remove`, etc.) | Directly modify the draft attachment and regenerate the proposal comment. See [Jira Labels & Comments](labels.md) for a list of commands. | +| **Ask a question** | Comment with `?` prefix or `@forge ask` | Forge answers your question without regenerating the draft. | +| **Request revisions** | Comment with `!` prefix followed by your feedback | Forge uses LLM assistance to revise the entire draft JSON and update the proposal comment with your feedback. | + +If `forge:yolo` mode is active, the draft review is bypassed. Epics are created in Jira immediately, and the workflow automatically proceeds to Task Generation. + +If `forge:direct-mode` is active, the draft review is also bypassed and Epics are created in Jira immediately, but the workflow still pauses at the `plan_approval_gate` waiting for manual human approval (via label or commands) before proceeding. ```mermaid flowchart TD Gate([plan_approval_gate]) - Gate -->|forge:plan-approved| Next[Generate Tasks] + Gate -->|forge:plan-approved or /forge approve| Next[Generate Tasks] Gate -->|"? on feature ticket"| QA[Answer Question] Gate -->|"! on feature ticket"| Regen[Regenerate All Epics] - Gate -->|"! on epic sub-ticket"| Update[Update Single Epic] + Gate -->|"/forge update/remove/exclude/add"| Update[Modify Draft] QA --> Gate Regen --> Gate Update --> Gate @@ -93,22 +102,31 @@ flowchart TD Forge generates granular implementation tasks scoped to individual repositories. Each task is sized to fit in a single container execution pass. -**Human action:** Review the tasks. You have four options at this stage: +By default, Forge uses an interactive **Draft Review Flow** at this stage (unless YOLO mode is active): +1. Instead of creating Jira tickets immediately, Forge serializes the proposed tasks into `forge-tasks-draft.json` and uploads it as an attachment on the Feature ticket. +2. Forge posts a markdown table comment on the Feature ticket outlining the proposed Tasks. +3. The workflow pauses at `task_approval_gate`. + +**Human action:** Review the task draft. You have several options at this stage: + +| Action | How | Description | +|--------|-----|-------------| +| **Approve** | Comment `/forge approve` OR set label to `forge:task-approved` | Forge provisions the Task sub-tickets on Jira from the draft, deletes the draft attachment, and advances to Implementation. | +| **Direct Edit** | Use `/forge` commands (e.g. `/forge update`, `/forge remove`, etc.) | Directly modify the draft attachment and regenerate the proposal comment. See [Jira Labels & Comments](labels.md) for a list of commands. | +| **Ask a question** | Comment with `?` prefix or `@forge ask` | Forge answers your question without regenerating the draft. | +| **Request revisions** | Comment with `!` prefix followed by your feedback | Forge uses LLM assistance to revise the entire draft JSON and update the proposal comment with your feedback. | + +If `forge:yolo` mode is active, the draft review is bypassed. Tasks are created in Jira immediately, and the workflow automatically proceeds to Implementation. -| Action | How | -|--------|-----| -| Approve | Change label to `forge:task-approved` | -| Ask a question | Comment with `?` prefix — Forge answers without regenerating | -| Revise one task | `!` comment on the **specific task sub-ticket** — Forge updates only that task | -| Regenerate all tasks | `!` comment on the **feature or epic ticket** — Forge regenerates the full task list with your feedback | +If `forge:direct-mode` is active, the draft review is also bypassed and Tasks are created in Jira immediately, but the workflow still pauses at the `task_approval_gate` waiting for manual human approval (via label or commands) before proceeding. ```mermaid flowchart TD Gate([task_approval_gate]) - Gate -->|forge:task-approved| Next[Implement Tasks] + Gate -->|forge:task-approved or /forge approve| Next[Implement Tasks] Gate -->|"? on ticket"| QA[Answer Question] Gate -->|"! on feature/epic"| Regen[Regenerate All Tasks] - Gate -->|"! on task sub-ticket"| Update[Update Single Task] + Gate -->|"/forge update/remove/exclude/add"| Update[Modify Draft] QA --> Gate Regen --> Gate Update --> Gate diff --git a/docs/guide/labels.md b/docs/guide/labels.md index a56e27eb2..66f86af61 100644 --- a/docs/guide/labels.md +++ b/docs/guide/labels.md @@ -45,19 +45,40 @@ Standalone Tasks and Epics can be processed with the standard `forge:managed` la | `forge:blocked` | Set by Forge when a stage fails. Forge posts a comment with the error. | | `forge:retry` | Add this to resume from the exact node that failed, or to transition from `review_response_gate` back to `human_review_gate` (clearing contested review comments). Forge removes it after resuming. | | `forge:yolo` | Auto-approve supported planning gates. Human PR review still remains a gate. | +| `forge:direct-mode` | Direct ticket creation mode (bypasses draft JSON attachments and creates Epic/Task tickets immediately in Jira), but still pauses for human approval at the planning gates (instead of auto-approving like `forge:yolo`). | | `repo:/` | Identifies repositories selected for planning and implementation. | ## How to Use Labels **Starting a workflow:** Create a Jira issue and add `forge:managed`. Forge detects the issue type and begins the appropriate pipeline: Feature/Story, Bug, or standalone Task/Epic takeover. -**Approving a stage:** When Forge posts a PRD, spec, or other artifact, it sets the `forge:*-pending` label. Change it to `forge:*-approved` to advance the workflow. Do not add the approved label manually before Forge posts — it won't be recognized until the pending state is set. +**Approving a stage:** When Forge posts an artifact (such as a PRD or Spec), it sets the `forge:*-pending` label. You can approve it by changing the label to `forge:*-approved` to advance the workflow. For draft-based stages (Epic Plan and Tasks), you can also approve by commenting `/forge approve` on the ticket. -**Requesting revisions:** Start a comment with `!` followed by your feedback. Forge regenerates the artifact and resets the pending label. +**Interactive Draft Review:** For Epic Decomposition and Task Generation stages, Forge uses a draft-based review flow by default (unless `forge:yolo` or `forge:direct-mode` mode is active). +1. Instead of creating sub-tickets immediately, Forge serializes the proposed items into a JSON draft file (`forge-epics-draft.json` or `forge-tasks-draft.json`) and uploads it as a Jira attachment. +2. Forge posts a formatted markdown table comment on the ticket detailing the proposed plan. +3. While the stage is pending, you can modify the draft directly using **Jira comment commands** (see below) or request a natural language revision. +4. Once you approve (via `/forge approve` or setting the approved label), Forge downloads the draft, provisions the actual Jira tickets from it, and deletes the draft attachment. -**Asking questions:** Start a comment with `?` or `@forge ask`. Forge answers without advancing or regenerating. +### Jira Comment Commands -**Informational comments:** Comments without a recognized prefix (`!`, `?`, `@forge ask`, `>option`) are ignored by the workflow — use them for team discussion without triggering Forge. +For stages using the draft-based review flow (Epic Plan and Tasks), you can post comments on the parent ticket with the following commands: + +| Command | Description | Example | +|---------|-------------|---------| +| `/forge approve` | Approve the draft, provision all non-excluded items as Jira tickets, and delete the draft attachment. | `/forge approve` | +| `/forge remove ` | Remove a draft item by its local sequential ID. Remaining items are automatically re-sequenced. | `/forge remove 3` | +| `/forge exclude ` | Toggle the exclusion flag of a draft item. Excluded items are skipped during ticket provisioning. | `/forge exclude 2` | +| `/forge update key=val` | Update fields of a draft item (supported keys: `summary`, `description`, `repo`). | `/forge update 1 repo="my-org/custom-repo"` | +| `/forge add key=val` | Add a new proposed item to the draft. | `/forge add summary="New Story" repo="my-org/repo"` | + +*Note: Successful command/revision comments are automatically edited by Forge to prepend `✅`. If a command or revision fails, Forge posts a comment detailing the error with a leading `❌`.* + +**Requesting revisions:** Start a comment with `!` followed by your feedback (e.g., `! update the repositories to use the new service`). For standard artifacts, Forge regenerates them. For drafts, Forge uses LLM assistance to revise the draft JSON attachment and update the proposed plan table. + +**Asking questions:** Start a comment with `?` or `@forge ask`. Forge answers without advancing or regenerating/modifying the drafts. + +**Informational comments:** Comments without a recognized prefix (such as `!`, `?`, `@forge ask`, `>option`, or `/forge`) are ignored by the workflow — use them for team discussion without triggering Forge. **Handling failures:** When `forge:blocked` appears, read the Forge comment for the error. Fix the underlying issue if needed, then add `forge:retry`. diff --git a/src/forge/integrations/agents/agent.py b/src/forge/integrations/agents/agent.py index cba44a79b..03a82f48f 100644 --- a/src/forge/integrations/agents/agent.py +++ b/src/forge/integrations/agents/agent.py @@ -1333,6 +1333,88 @@ async def answer_question( logger.info(f"Generated answer ({len(result)} chars)") return result.strip() if result else "" + async def revise_draft_with_feedback( + self, + draft_content: str, + feedback: str, + context: dict[str, Any] | None = None, + ) -> str: + """Revise draft content based on user feedback. + + Uses the 'revision-draft' prompt template to guide the LLM to output + the revised draft JSON. + + Args: + draft_content: The current draft JSON content. + feedback: Natural language feedback. + context: Optional context from the workflow state. + + Returns: + The updated draft JSON string. + """ + from langchain_core.output_parsers import StrOutputParser + + # Format context into a readable string/JSON + context_str = json.dumps(context, indent=2) if context else "None provided" + + # Load the prompt template using project's load_prompt + prompt_text = load_prompt( + "revision-draft", + draft_content=draft_content, + feedback=feedback, + context=context_str, + ) + + model = self._create_model() + chain = model | StrOutputParser() + + logger.info("Revising draft using direct LangChain model chain") + response = await chain.ainvoke(prompt_text) + + # Strip preamble/narration and validate as JSON + cleaned_text = response.strip() + + # Check markdown code blocks first + pattern = r"```(?:json)?\s*([\s\S]*?)\s*```" + match = re.search(pattern, cleaned_text) + if match: + cleaned_text = match.group(1).strip() + else: + # If no code block, look for the JSON object/list boundary + # Find the first occurrence of '{' or '[' and the last of '}' or ']' + start_brace = cleaned_text.find("{") + start_bracket = cleaned_text.find("[") + + # Determine which starts first + start_idx = -1 + if start_brace != -1 and start_bracket != -1: + start_idx = min(start_brace, start_bracket) + elif start_brace != -1: + start_idx = start_brace + elif start_bracket != -1: + start_idx = start_bracket + + if start_idx != -1: + # Find the last brace or bracket matching the start type + if start_idx == start_brace: + end_idx = cleaned_text.rfind("}") + else: + end_idx = cleaned_text.rfind("]") + + if end_idx > start_idx: + cleaned_text = cleaned_text[start_idx : end_idx + 1].strip() + + try: + parsed_json = json.loads(cleaned_text) + validated_json_str = json.dumps(parsed_json, indent=2) + logger.info( + f"Successfully revised draft and validated JSON ({len(validated_json_str)} chars)" + ) + return validated_json_str + except json.JSONDecodeError as e: + logger.error(f"Failed to parse LLM response as valid JSON: {e}\nResponse: {response}") + raise ValueError(f"Failed to parse revised draft as JSON: {e}") + async def close(self) -> None: """Close the agent and cleanup resources.""" pass diff --git a/src/forge/integrations/jira/client.py b/src/forge/integrations/jira/client.py index 02e4dcc5a..a62503af0 100644 --- a/src/forge/integrations/jira/client.py +++ b/src/forge/integrations/jira/client.py @@ -4,7 +4,7 @@ import json import logging import re -from typing import Any +from typing import Any, cast import httpx from pydantic import ValidationError @@ -85,7 +85,6 @@ async def _get_client(self) -> httpx.AsyncClient: ), headers={ "Accept": "application/json", - "Content-Type": "application/json", }, timeout=30.0, ) @@ -419,58 +418,86 @@ async def add_attachment( Args: issue_key: The Jira issue key. filename: Name for the attachment file. - content: File content (string or bytes). - content_type: MIME type of the content. + content: File content as string or bytes. + content_type: The content type of the file. Returns: The attachment metadata from Jira API. """ - # Attachments require a separate client without JSON content-type - async with httpx.AsyncClient( - base_url=self.base_url, - auth=( - self.settings.jira_user_email, - self.settings.jira_api_token.get_secret_value(), - ), - headers={ - "Accept": "application/json", - "X-Atlassian-Token": "no-check", # Required for attachments - }, - timeout=60.0, - ) as client: - # Convert string to bytes if needed - if isinstance(content, str): - content = content.encode("utf-8") - - files = {"file": (filename, content, content_type)} - response = await client.post( - f"/issue/{issue_key}/attachments", - files=files, - ) - response.raise_for_status() - data = response.json() - logger.info(f"Added attachment {filename} to {issue_key}") - return data[0] if data else {} + if isinstance(content, str): + content = content.encode("utf-8") + + if content_type == "text/markdown" and filename.endswith(".json"): + content_type = "application/json" + + headers = { + "X-Atlassian-Token": "no-check", + } + files = {"file": (filename, content, content_type)} + + response = await self._request_with_retry( + "POST", + f"/issue/{issue_key}/attachments", + headers=headers, + files=files, + ) + response.raise_for_status() + data = response.json() + logger.info(f"Added attachment {filename} to {issue_key}") + return data[0] if data else {} async def get_attachments(self, issue_key: str) -> list[dict[str, Any]]: - """Get all attachments for a Jira issue. + """Get all attachments for a Jira issue by querying the issue's details. Args: issue_key: The Jira issue key. Returns: - List of attachment metadata dicts with 'id', 'filename', 'size', etc. + A list of attachment metadata dicts containing id, filename, and content URL. """ - client = await self._get_client() - response = await client.get( + response = await self._request_with_retry( + "GET", f"/issue/{issue_key}", params={"fields": "attachment"}, ) response.raise_for_status() data = response.json() attachments = data.get("fields", {}).get("attachment", []) - logger.debug(f"Found {len(attachments)} attachments on {issue_key}") - return attachments + + result = [] + for att in attachments: + result.append( + { + "id": att.get("id"), + "filename": att.get("filename"), + "content_url": att.get("content"), + } + ) + logger.debug(f"Found {len(result)} attachments on {issue_key}") + return result + + async def download_attachment(self, content_url: str) -> bytes: + """Download attachment raw binary content from the given content URL. + + Args: + content_url: The full URL to download the attachment. + + Returns: + The raw binary content of the attachment. + """ + response = await self._request_with_retry("GET", content_url, follow_redirects=False) + if response.status_code in (301, 302, 303, 307, 308): + redirect_url = response.headers.get("Location") + if not redirect_url: + raise ValueError("Redirect response missing Location header") + logger.info("Downloading attachment securely via unauthenticated redirect") + async with httpx.AsyncClient(follow_redirects=True) as anon_client: + anon_response = await anon_client.get(redirect_url) + anon_response.raise_for_status() + return anon_response.content + + response.raise_for_status() + return response.content async def delete_attachment(self, attachment_id: str) -> None: """Delete an attachment by ID. @@ -478,8 +505,7 @@ async def delete_attachment(self, attachment_id: str) -> None: Args: attachment_id: The Jira attachment ID. """ - client = await self._get_client() - response = await client.delete(f"/attachment/{attachment_id}") + response = await self._request_with_retry("DELETE", f"/attachment/{attachment_id}") response.raise_for_status() logger.info(f"Deleted attachment {attachment_id}") @@ -591,6 +617,11 @@ async def add_comment(self, issue_key: str, body: str) -> JiraComment: Returns: The created JiraComment. """ + if len(body) > 32767: + raise ValueError( + f"Comment body length ({len(body)}) exceeds maximum Jira limit of 32767 characters" + ) + client = await self._get_client() adf_content = self._text_to_adf(body) @@ -1350,7 +1381,7 @@ def _text_to_adf(text: str) -> dict[str, Any]: } try: - adf = convert(text) + adf = convert(JiraClient._prepare_markdown_for_adf(text)) except Exception as e: logger.warning(f"ADF conversion failed, using simple fallback: {e}") # Simple fallback - just paragraphs @@ -1371,7 +1402,57 @@ def _text_to_adf(text: str) -> dict[str, Any]: } JiraClient._link_bare_urls(adf) - return adf + return cast(dict[str, Any], adf) + + @staticmethod + def _prepare_markdown_for_adf(text: str) -> str: + """Preserve intentional newlines unsupported by ``md-to-adf``. + + Agent-generated plans commonly use one logical step per line without + inserting Markdown blank lines. ``md-to-adf`` joins such lines with a + space, producing a single dense Jira paragraph. Separate consecutive + prose lines while leaving fenced code, tables, and list structures + untouched. + """ + lines = text.split("\n") + table_lines: set[int] = set() + for index in range(len(lines) - 1): + if "|" not in lines[index]: + continue + if re.fullmatch(r"[\s|:-]+", lines[index + 1]): + table_lines.update({index, index + 1}) + row_index = index + 2 + while ( + row_index < len(lines) and "|" in lines[row_index] and lines[row_index].strip() + ): + table_lines.add(row_index) + row_index += 1 + + prepared: list[str] = [] + in_fence = False + list_line = re.compile(r"^\s*(?:[-*+] |\d+[.)] )") + + for index, line in enumerate(lines): + prepared.append(line) + if line.strip().startswith("```"): + in_fence = not in_fence + continue + if in_fence or index == len(lines) - 1: + continue + + next_line = lines[index + 1] + if not line.strip() or not next_line.strip(): + continue + if index in table_lines or index + 1 in table_lines: + continue + if list_line.match(line) or list_line.match(next_line): + continue + if line.startswith((" ", "\t")) or next_line.startswith((" ", "\t")): + continue + + prepared.append("") + + return "\n".join(prepared) @staticmethod def _link_bare_urls(node: dict[str, Any], *, in_code_block: bool = False) -> None: diff --git a/src/forge/models/__init__.py b/src/forge/models/__init__.py index 17a8b697f..d67d6881d 100644 --- a/src/forge/models/__init__.py +++ b/src/forge/models/__init__.py @@ -1,6 +1,7 @@ """Domain models for Forge orchestrator.""" from forge.models.artifacts import Epic, Feature, Task +from forge.models.draft import DraftItem, ForgeDecompositionDraft from forge.models.events import EventSource, EventStatus, WebhookEvent from forge.models.workflow import ( ForgeLabel, @@ -23,6 +24,8 @@ "Feature", "Epic", "Task", + "DraftItem", + "ForgeDecompositionDraft", # Event models "WebhookEvent", "EventSource", diff --git a/src/forge/models/draft.py b/src/forge/models/draft.py new file mode 100644 index 000000000..d868a5c5a --- /dev/null +++ b/src/forge/models/draft.py @@ -0,0 +1,77 @@ +"""Data models for decomposing draft artifacts.""" + +from datetime import datetime +from typing import Literal + +from pydantic import BaseModel, model_validator + + +class DraftItem(BaseModel): + """Represents an individual proposed Story or Task inside a draft.""" + + model_config = {"extra": "forbid"} + + id: int + """Local sequential ID, e.g., 1, 2, 3.""" + + summary: str + """Brief summary of the proposed item.""" + + description: str + """Detailed description of the proposed item.""" + + repo: str + """Target repository name.""" + + acceptance_criteria: list[str] + """List of acceptance criteria for this item.""" + + excluded: bool = False + """Whether this item should be excluded from ticket creation.""" + + epic_key: str | None = None + """Optional Jira key of the parent epic for this task.""" + + +class ForgeDecompositionDraft(BaseModel): + """Represents the wrapper of all draft items and execution metadata.""" + + parent_key: str + """Jira key of the parent feature or epic.""" + + phase: Literal["epics", "tasks"] + """Phase of the draft, either "epics" or "tasks".""" + + items: list[DraftItem] + """List of draft items (proposed Stories or Tasks).""" + + version: int = 1 + """Draft schema version.""" + + created_at: datetime + """Timestamp when this draft was created.""" + + updated_at: datetime + """Timestamp when this draft was last updated.""" + + @model_validator(mode="after") + def _validate_sequential_ids(self) -> "ForgeDecompositionDraft": + """Validate that local item IDs are unique and sequential (1, 2, 3...) within the draft.""" + if not self.items: + return self + + ids = [item.id for item in self.items] + + # Check uniqueness + if len(ids) != len(set(ids)): + raise ValueError("Draft item IDs must be unique.") + + # Check that IDs are sequential starting from 1 + sorted_ids = sorted(ids) + expected_ids = list(range(1, len(self.items) + 1)) + if sorted_ids != expected_ids: + raise ValueError( + f"Draft item IDs must be sequential starting from 1. Got: {sorted_ids}, expected: {expected_ids}" + ) + + return self diff --git a/src/forge/models/workflow.py b/src/forge/models/workflow.py index d27c4064a..38b96eab8 100644 --- a/src/forge/models/workflow.py +++ b/src/forge/models/workflow.py @@ -132,6 +132,7 @@ class ForgeLabel(StrEnum): BLOCKED = "forge:blocked" RETRY = "forge:retry" # Add to trigger retry of current stage YOLO = "forge:yolo" # Skip human approval gates — auto-approve all artifact reviews + DIRECT_MODE = "forge:direct-mode" class TicketType(StrEnum): diff --git a/src/forge/orchestrator/worker.py b/src/forge/orchestrator/worker.py index 2aee63e37..1d82d0bc4 100644 --- a/src/forge/orchestrator/worker.py +++ b/src/forge/orchestrator/worker.py @@ -9,8 +9,9 @@ import sys import uuid from dataclasses import replace as dataclass_replace +from datetime import UTC, datetime from pathlib import Path -from typing import Any +from typing import Any, cast from forge.api.routes.metrics import ( record_workflow_completed, @@ -18,6 +19,7 @@ record_workflow_started, ) from forge.config import get_settings +from forge.integrations.agents import ForgeAgent from forge.integrations.github.comment_signature import is_self_comment from forge.integrations.jira.client import JiraClient from forge.integrations.source_control.contracts import ( @@ -30,6 +32,7 @@ ReviewState, ) from forge.integrations.source_control.registry import get_registry +from forge.models.draft import DraftItem, ForgeDecompositionDraft from forge.models.events import EventSource from forge.models.workflow import ForgeLabel, TicketType from forge.orchestrator.checkpointer import get_checkpointer, get_ticket_from_pr_index @@ -43,6 +46,8 @@ selected_workflow_name, ) from forge.workflow.declarative.workflow import DeclarativeWorkflow +from forge.workflow.gates.plan_approval import provision_epics_from_draft +from forge.workflow.gates.task_approval import provision_tasks_from_draft from forge.workflow.nodes.error_handler import notify_error from forge.workflow.nodes.workspace_setup import teardown_workspace from forge.workflow.pr_state import ( @@ -54,11 +59,20 @@ ) from forge.workflow.registry import create_default_router from forge.workflow.router import WorkflowRouter +from forge.workflow.utils import check_direct_mode from forge.workflow.utils.automated_review_triage import ( is_bot_sender, triage_automated_review, ) -from forge.workflow.utils.comment_classifier import CommentType, classify_comment +from forge.workflow.utils.comment_classifier import ( + CommentType, + classify_comment, + parse_comment_command, +) +from forge.workflow.utils.draft_manager import ( + FORGE_EPICS_DRAFT_FILENAME, + DraftManager, +) from forge.workflow.utils.jira_status import post_status_comment from forge.workflow.utils.proposal_review_threads import ( reply_to_proposal_decisions, @@ -179,6 +193,12 @@ async def _cleanup_terminal_workspace(result: dict[str, Any]) -> dict[str, Any]: "rca_option_gate", } +_PENDING_APPROVAL_GATES = { + "plan_approval_gate", + "task_plan_approval_gate", + "task_approval_gate", +} + class OrchestratorWorker: """Worker that processes workflow events from Redis queue.""" @@ -553,6 +573,25 @@ async def _process_workflow(self, message: QueueMessage) -> None: if updated_values is existing_state.values: return + # Draft edits intentionally leave the approval gate paused. Persist + # those edits, but do not invoke the graph: ainvoke(None) would only + # re-enter the same gate and gives the misleading impression that the + # workflow was resumed. This is also the checkpoint write that makes + # a subsequent approval provision the draft the reviewer just saw. + if ( + updated_values.get("is_paused") + and updated_values.get("current_node") in _PENDING_APPROVAL_GATES + and updated_values.get("current_node") + == existing_state.values.get("current_node") + ): + logger.info( + "Persisting paused workflow update for %s at %s without resuming", + ticket_key, + updated_values.get("current_node"), + ) + await compiled_workflow.aupdate_state(config, updated_values) + return + logger.info(f"Resuming workflow for {ticket_key}") was_errored = _is_workflow_errored(existing_state.values) @@ -878,6 +917,24 @@ async def _handle_resume_event( is_retry = True logger.info(f"Detected retry signal via forge:retry label for {current_node}") + # Direct check for forge:plan-approved and forge:task-approved additions + to_lower = to_labels.lower() if to_labels else "" + from_lower = from_labels.lower() if from_labels else "" + if ( + "forge:plan-approved" in to_lower + and "forge:plan-approved" not in from_lower + and current_node in ("plan_approval_gate", "task_plan_approval_gate") + ): + is_approved = True + logger.info(f"Detected forge:plan-approved addition on {message.ticket_key}") + if ( + "forge:task-approved" in to_lower + and "forge:task-approved" not in from_lower + and current_node == "task_approval_gate" + ): + is_approved = True + logger.info(f"Detected forge:task-approved addition on {message.ticket_key}") + # Check for approval labels - but only if it matches the current stage if "approved" in to_labels.lower() and "pending" in from_labels.lower(): # Validate the approval matches the workflow stage @@ -962,7 +1019,273 @@ async def _handle_resume_event( if isinstance(comment_body, dict): comment_body = self._extract_text_from_adf(comment_body) + # Child issue webhooks are routed to the parent Feature workflow. + # Keep the issue where the human actually commented as the target + # for draft slicing and all Jira replies/edits. + interaction_ticket_key = payload.get("source_ticket_key") or message.ticket_key + if comment_body.strip(): + # Check for interactive comment commands or natural language feedback when paused in PENDING_APPROVAL (BR-006) + if current_state.get("is_paused") and current_node in _PENDING_APPROVAL_GATES: + parsed_cmd = parse_comment_command(comment_body) + is_forge_cmd = parsed_cmd is not None + # Revision comment check allows leading whitespace for consistency with the comment classifier + is_revision_comment = bool(re.match(r"^\s*!", comment_body)) + + is_direct_mode = check_direct_mode(current_state) + if not is_direct_mode and (is_forge_cmd or is_revision_comment): + draft_key = ( + "plan_draft" + if current_node in ("plan_approval_gate", "task_plan_approval_gate") + else "tasks_draft" + ) + + jira = JiraClient() + try: + # 1. Handle parsing errors immediately + if is_forge_cmd and parsed_cmd is not None and "error" in parsed_cmd: + await jira.add_comment( + interaction_ticket_key, + f"❌ **Error parsing command:** {parsed_cmd['error']}", + ) + await jira.close() + return current_state + + original_draft_raw = current_state.get(draft_key) + original_draft = None + if original_draft_raw is not None: + if isinstance(original_draft_raw, dict): + original_draft = ForgeDecompositionDraft.model_validate( + original_draft_raw + ) + else: + original_draft = original_draft_raw + + # 2. Slice aggregate_draft if we are on an Epic (child ticket) + if ( + draft_key == "tasks_draft" + and interaction_ticket_key != current_state.get("ticket_key") + ): + epic_items = [] + if original_draft: + epic_items = [ + item.model_copy() + for item in original_draft.items + if item.epic_key == interaction_ticket_key + ] + # Re-sequence local IDs + for idx, item in enumerate(epic_items, start=1): + item.id = idx + + original_draft = ForgeDecompositionDraft( + parent_key=interaction_ticket_key, + phase="tasks", + items=epic_items, + version=original_draft.version if original_draft else 1, + created_at=original_draft.created_at + if original_draft + else datetime.now(UTC), + updated_at=original_draft.updated_at + if original_draft + else datetime.now(UTC), + ) + + if original_draft is None or ( + original_draft_raw is None and not original_draft.items + ): + # Workflows checkpointed before draft state was persisted + # still use the legacy revision signal. Let the graph + # regenerate their draft instead of attempting an LLM + # edit with no source document. + if is_revision_comment: + feedback_text = re.sub(r"^\s*!\s*", "", comment_body) + await post_status_comment( + jira, + interaction_ticket_key, + "✅ Forge received your revision request " + f"from {interaction_ticket_key}.", + ) + await jira.close() + return { + **current_state, + "revision_requested": True, + "feedback_comment": feedback_text, + "current_epic_key": ( + interaction_ticket_key + if interaction_ticket_key + != current_state.get("ticket_key") + else None + ), + "current_task_key": None, + } + raise ValueError(f"Draft '{draft_key}' not found in state.") + + updated_draft = None + if is_forge_cmd and parsed_cmd is not None: + draft_json = [item.model_dump() for item in original_draft.items] + mutated_json = DraftManager.apply_draft_modification( + draft_json, parsed_cmd + ) + + updated_items = [ + DraftItem.model_validate(item) for item in mutated_json + ] + updated_draft = ForgeDecompositionDraft( + parent_key=original_draft.parent_key, + phase=original_draft.phase, + items=updated_items, + version=original_draft.version, + created_at=original_draft.created_at, + updated_at=datetime.now(UTC), + ) + + elif is_revision_comment: + feedback_text = re.sub(r"^\s*!\s*", "", comment_body) + if not feedback_text: + raise ValueError("Revision feedback cannot be empty.") + + feature_key = current_state.get("ticket_key") or message.ticket_key + feature_issue = await jira.get_issue(feature_key) + feature_summary = "" + feature_description = "" + if feature_issue: + if hasattr(feature_issue, "summary"): + feature_summary = str(feature_issue.summary) + if hasattr(feature_issue, "description"): + feature_description = str(feature_issue.description) + + context_data = { + "parent_key": feature_key, + "parent_summary": feature_summary, + "parent_description": feature_description, + "prd": current_state.get("prd_content", ""), + "spec": current_state.get("spec_content", ""), + "ticket_key": interaction_ticket_key, + "current_node": current_node, + } + + agent = ForgeAgent() + try: + revised_json_str = await agent.revise_draft_with_feedback( + draft_content=original_draft.model_dump_json(), + feedback=feedback_text, + context=context_data, + ) + finally: + await agent.close() + + updated_draft = ForgeDecompositionDraft.model_validate_json( + revised_json_str + ) + + if updated_draft is not None: + # Update Feature state with the modified draft + if ( + draft_key == "tasks_draft" + and interaction_ticket_key != current_state.get("ticket_key") + ): + # It's an Epic slice update. Merge back. + aggregate_draft = current_state.get("tasks_draft") + if aggregate_draft: + if isinstance(aggregate_draft, dict): + aggregate_draft = ( + ForgeDecompositionDraft.model_validate( + aggregate_draft + ) + ) + other_items = [ + item.model_copy() + for item in aggregate_draft.items + if item.epic_key != interaction_ticket_key + ] + updated_epic_items = [] + for item in updated_draft.items: + cloned_item = item.model_copy() + cloned_item.epic_key = interaction_ticket_key + updated_epic_items.append(cloned_item) + + combined_items = other_items + updated_epic_items + + for idx, item in enumerate(combined_items, start=1): + item.id = idx + + updated_aggregate_draft = ForgeDecompositionDraft( + parent_key=current_state.get("ticket_key"), + phase="tasks", + items=combined_items, + version=aggregate_draft.version, + created_at=aggregate_draft.created_at, + updated_at=datetime.now(UTC), + ) + else: + combined_items = [] + for idx, item in enumerate(updated_draft.items, start=1): + cloned_item = item.model_copy() + cloned_item.epic_key = interaction_ticket_key + cloned_item.id = idx + combined_items.append(cloned_item) + + updated_aggregate_draft = ForgeDecompositionDraft( + parent_key=current_state.get("ticket_key"), + phase="tasks", + items=combined_items, + version=updated_draft.version, + created_at=updated_draft.created_at, + updated_at=datetime.now(UTC), + ) + + current_state["tasks_draft"] = updated_aggregate_draft + else: + current_state[draft_key] = updated_draft + if draft_key == "plan_draft": + try: + await DraftManager.save_draft_attachment( + jira, + current_state.get("ticket_key") + or message.ticket_key, + updated_draft, + FORGE_EPICS_DRAFT_FILENAME, + ) + except Exception as write_err: + logger.warning( + f"Failed one-way draft write: {write_err}" + ) + + # Preserve the original proposal and publish the revised + # breakdown as a new comment so the planning history remains + # visible to reviewers. + await self._post_updated_review_comment( + jira, interaction_ticket_key, updated_draft + ) + + # Do not alter the reviewer's command or feedback either. + # An acknowledgement is a separate event in the discussion. + await jira.add_comment( + interaction_ticket_key, + f"✅ {comment_body}", + ) + + await jira.close() + # A new mapping is the resume signal for draft mutations. + # The worker uses identity to distinguish these updates from + # unrelated comments while a workflow is paused. + return {**current_state} + + except Exception as e: + logger.error( + f"Failed to process comment command/revision: {e}", + exc_info=True, + ) + try: + await jira.add_comment( + interaction_ticket_key, + f"❌ **Error processing command/revision:** {e}", + ) + except Exception as post_err: + logger.error(f"Failed to post error comment: {post_err}") + await jira.close() + return current_state + # >option N detection for rca_option_gate (runs before general classification) if current_node == "rca_option_gate": option_match = _OPTION_PATTERN.search(comment_body) @@ -1333,7 +1656,7 @@ async def _handle_resume_event( else None ) pr_number = int(native_id) if native_id is not None else None - inline_comments: list[dict[str, Any]] = [] + inline_comments = [] if repo_full and pr_number: _repo_ref_obj, _adapter = get_adapter(repo_full) _identity = identity_for(_repo_ref_obj, pr_number) @@ -1821,6 +2144,54 @@ async def _handle_resume_event( updated_state["automated_review_revision_count"] = 0 updated_state["automated_review_revision_pending"] = False updated_state["proposal_review_decisions"] = [] + + # Ticket provisioning step on approval! + # Note on split ownership: The worker call site handles unpausing from human manual comments (webhook triggers) + # to catch and report provisioning errors early without breaking the LangGraph execution flow. + if current_node == "plan_approval_gate" and not updated_state.get("epic_keys"): + jira = JiraClient() + try: + epic_keys = await provision_epics_from_draft(cast(Any, updated_state), jira) + updated_state["epic_keys"] = epic_keys + except Exception as e: + logger.error( + f"Failed ticket provisioning during plan approval for {message.ticket_key}: {e}", + exc_info=True, + ) + # Keep paused in PENDING_APPROVAL and post error comment + error_comment_text = f"❌ Ticket provisioning failed: {redact_secrets(e)}" + try: + await jira.add_comment(message.ticket_key, error_comment_text) + except Exception as post_err: + logger.error(f"Failed to post error comment: {post_err}", exc_info=True) + return current_state + finally: + await jira.close() + + elif current_node == "task_approval_gate" and not updated_state.get("task_keys"): + # Note on split ownership: The worker call site handles unpausing from human manual comments (webhook triggers) + # to catch and report provisioning errors early without breaking the LangGraph execution flow. + jira = JiraClient() + try: + task_keys, tasks_by_repo = await provision_tasks_from_draft( + cast(Any, updated_state), jira + ) + updated_state["task_keys"] = task_keys + updated_state["tasks_by_repo"] = tasks_by_repo + except Exception as e: + logger.error( + f"Failed ticket provisioning during task approval for {message.ticket_key}: {e}", + exc_info=True, + ) + # Keep paused in PENDING_APPROVAL and post error comment + error_comment_text = f"❌ Ticket provisioning failed: {redact_secrets(e)}" + try: + await jira.add_comment(message.ticket_key, error_comment_text) + except Exception as post_err: + logger.error(f"Failed to post error comment: {post_err}", exc_info=True) + return current_state + finally: + await jira.close() elif is_question: # Unpause so answer_question node runs, it will re-pause after answering updated_state["is_paused"] = False @@ -2110,6 +2481,22 @@ async def _post_rebase_feedback( except Exception as e: logger.warning(f"Failed to post rebase feedback: {e}") + async def _post_updated_review_comment( + self, jira: JiraClient, ticket_key: str, draft: ForgeDecompositionDraft + ) -> None: + """Post a revised planning breakdown without changing prior comments. + + Args: + jira: The Jira client. + ticket_key: The Jira ticket key. + draft: The updated draft decomposition. + """ + try: + await jira.add_comment(ticket_key, DraftManager.format_review_comment(draft)) + logger.info("Posted revised %s draft comment on %s", draft.phase, ticket_key) + except Exception as c_err: + logger.warning(f"Could not post revised review comment: {c_err}") + async def _post_terminal_error_comment(self, ticket_key: str, error: str) -> None: """Post a comment explaining how to retry a terminal error. @@ -2341,6 +2728,7 @@ def _build_initial_state( ) yolo_mode = ForgeLabel.YOLO in labels + direct_mode = ForgeLabel.DIRECT_MODE in labels event_state = { "ticket_key": message.ticket_key, @@ -2355,6 +2743,7 @@ def _build_initial_state( "is_paused": False, "retry_count": message.retry_count, "yolo_mode": yolo_mode, + "direct_mode": direct_mode, } if isinstance(workflow_instance, DeclarativeWorkflow): initial = workflow_instance.create_initial_state(message.ticket_key) diff --git a/src/forge/prompts/v1/revision-draft.md b/src/forge/prompts/v1/revision-draft.md new file mode 100644 index 000000000..672627f4e --- /dev/null +++ b/src/forge/prompts/v1/revision-draft.md @@ -0,0 +1,22 @@ +Please revise the following draft JSON list based on the parent issue context and natural language feedback. + +## Current Draft Content (JSON) + +{draft_content} + +## Parent Issue Context + +{context} + +## Feedback / Revision Request + +{feedback} + +## Instructions + +- Revise the draft JSON list to incorporate the feedback. +- Preserve the existing JSON structure and fields. +- Ensure all items in the draft maintain valid structure and formatting. +- Make sure the output is a valid JSON string representing the updated draft. +- You MUST output ONLY the raw JSON string. Do not include any preamble, introduction, explanation, or markdown code block syntax (like ```json ... ```). +- Start your response directly with the opening curly brace `{` or square bracket `[`, ensuring your response is a single complete JSON object or array (for example `{ ... }` or `[ ... ]`) without markdown formatting. diff --git a/src/forge/sandbox/runner.py b/src/forge/sandbox/runner.py index cf3063e56..9c91c06e0 100644 --- a/src/forge/sandbox/runner.py +++ b/src/forge/sandbox/runner.py @@ -240,7 +240,7 @@ def _build_env_vars( Returns: Dict of environment variables. """ - env = {} + env: dict[str, str] = {} selected_backend = model_target.backend if model_target else self.settings.llm_backend if not selected_backend: @@ -586,7 +586,7 @@ async def _start_review_polling( task_key: str, skill_name: str, collected_cycles: list[ReviewCycleData], - ) -> tuple[ReviewCyclePoller | None, ReviewCycleRecorder | None, asyncio.Task | None]: + ) -> tuple[ReviewCyclePoller | None, ReviewCycleRecorder | None, asyncio.Task[Any] | None]: """Create review poller, recorder, and start background polling task. Args: @@ -659,7 +659,7 @@ async def _finalize_review_polling( self, poller: ReviewCyclePoller | None, recorder: ReviewCycleRecorder | None, - polling_task: asyncio.Task | None, + polling_task: asyncio.Task[Any] | None, workspace_path: Path, step_name: str | None, task_key: str, @@ -877,7 +877,7 @@ async def run( collected_cycles: list[ReviewCycleData] = [] poller: ReviewCyclePoller | None = None recorder: ReviewCycleRecorder | None = None - polling_task: asyncio.Task | None = None + polling_task: asyncio.Task[Any] | None = None try: # Build container name and execution spec diff --git a/src/forge/workflow/base.py b/src/forge/workflow/base.py index 4480e70b8..301042b6e 100644 --- a/src/forge/workflow/base.py +++ b/src/forge/workflow/base.py @@ -119,6 +119,7 @@ class BaseState(TypedDict, total=False): feedback_comment: str | None revision_requested: bool yolo_mode: bool # When True, approval gates auto-pass without human input + direct_mode: bool # When True, directly provision tickets instead of drafting # Message history messages: Annotated[list[Any], add_messages] diff --git a/src/forge/workflow/bug/state.py b/src/forge/workflow/bug/state.py index 1b9085f8c..cb5f2d5c5 100644 --- a/src/forge/workflow/bug/state.py +++ b/src/forge/workflow/bug/state.py @@ -157,6 +157,7 @@ def create_initial_bug_state(ticket_key: str, **kwargs: Any) -> BugState: "qualitative_review_failed": False, "reflect_rca_retry_count": 0, "yolo_mode": False, + "direct_mode": False, } # Merge with kwargs, letting kwargs override defaults diff --git a/src/forge/workflow/feature/graph.py b/src/forge/workflow/feature/graph.py index bd8f56f9b..3fddfe178 100644 --- a/src/forge/workflow/feature/graph.py +++ b/src/forge/workflow/feature/graph.py @@ -12,6 +12,8 @@ from forge.workflow.gates import ( plan_approval_gate, prd_approval_gate, + provision_epics, + provision_tasks, route_plan_approval, route_prd_approval, route_spec_approval, @@ -105,6 +107,8 @@ def route_by_ticket_type(state: FeatureState) -> str: return "update_single_epic" elif current_node == "plan_approval_gate": return "plan_approval_gate" + elif current_node == "provision_epics": + return "provision_epics" elif current_node == "generate_tasks": return "generate_tasks" elif current_node == "regenerate_all_tasks": @@ -115,6 +119,10 @@ def route_by_ticket_type(state: FeatureState) -> str: return "regenerate_epic_tasks" elif current_node == "task_approval_gate": return "task_approval_gate" + elif current_node == "provision_tasks": + return "provision_tasks" + elif current_node == "wait_for_ci_gate": + return "wait_for_ci_gate" elif current_node in ("implement_task", "implementation", "implement_bug_fix"): return "implement_task" elif current_node == "setup_workspace": @@ -395,6 +403,7 @@ def build_feature_graph() -> StateGraph: graph.add_node("plan_approval_gate", plan_approval_gate) graph.add_node("regenerate_all_epics", regenerate_all_epics) graph.add_node("update_single_epic", update_single_epic) + graph.add_node("provision_epics", provision_epics) # Task Generation nodes (US4) graph.add_node("generate_tasks", generate_tasks) @@ -402,6 +411,7 @@ def build_feature_graph() -> StateGraph: graph.add_node("regenerate_all_tasks", regenerate_all_tasks) graph.add_node("update_single_task", update_single_task) graph.add_node("regenerate_epic_tasks", regenerate_epic_tasks) + graph.add_node("provision_tasks", provision_tasks) # Execution nodes (US6) graph.add_node("task_router", route_tasks_by_repo) @@ -447,11 +457,13 @@ def build_feature_graph() -> StateGraph: "regenerate_all_epics": "regenerate_all_epics", "update_single_epic": "update_single_epic", "plan_approval_gate": "plan_approval_gate", + "provision_epics": "provision_epics", "generate_tasks": "generate_tasks", "regenerate_all_tasks": "regenerate_all_tasks", "update_single_task": "update_single_task", "regenerate_epic_tasks": "regenerate_epic_tasks", "task_approval_gate": "task_approval_gate", + "provision_tasks": "provision_tasks", # Resume routing for Feature workflow - execution stages "task_router": "task_router", "setup_workspace": "setup_workspace", @@ -545,13 +557,15 @@ def build_feature_graph() -> StateGraph: "plan_approval_gate", route_plan_approval, { - "generate_tasks": "generate_tasks", + "decompose_epics": "decompose_epics", + "provision_epics": "provision_epics", "regenerate_all_epics": "regenerate_all_epics", "update_single_epic": "update_single_epic", "answer_question": "answer_question", # Q&A mode END: END, # Pause workflow until next webhook }, ) + graph.add_edge("provision_epics", "generate_tasks") graph.add_conditional_edges( "regenerate_all_epics", _route_after_epic_regeneration, @@ -582,7 +596,8 @@ def build_feature_graph() -> StateGraph: "task_approval_gate", route_task_approval, { - "task_router": "task_router", + "generate_tasks": "generate_tasks", + "provision_tasks": "provision_tasks", "regenerate_all_tasks": "regenerate_all_tasks", # Feature-level rejection "regenerate_epic_tasks": "regenerate_epic_tasks", # Epic-level rejection "update_single_task": "update_single_task", # Task-level rejection @@ -590,6 +605,7 @@ def build_feature_graph() -> StateGraph: END: END, # Pause workflow until approval webhook }, ) + graph.add_edge("provision_tasks", "task_router") graph.add_conditional_edges( "regenerate_all_tasks", _route_after_task_regeneration, diff --git a/src/forge/workflow/feature/state.py b/src/forge/workflow/feature/state.py index 04e3ac89d..df346a39c 100644 --- a/src/forge/workflow/feature/state.py +++ b/src/forge/workflow/feature/state.py @@ -4,6 +4,7 @@ from typing import Any, cast from forge.config import get_settings +from forge.models.draft import ForgeDecompositionDraft from forge.models.workflow import TicketType from forge.workflow.base import ( BaseState, @@ -69,6 +70,10 @@ class FeatureState( spec_pr_branch: str | None spec_pr_file_path: str | None + # Draft state tracking + plan_draft: ForgeDecompositionDraft | None + tasks_draft: ForgeDecompositionDraft | None + def create_initial_feature_state(ticket_key: str, **kwargs: Any) -> FeatureState: """Create initial state for a new Feature workflow run.""" @@ -167,6 +172,9 @@ def create_initial_feature_state(ticket_key: str, **kwargs: Any) -> FeatureState "spec_pr_branch": None, "spec_pr_file_path": None, "yolo_mode": False, + "direct_mode": False, + "plan_draft": None, + "tasks_draft": None, } # Merge with kwargs, letting kwargs override defaults diff --git a/src/forge/workflow/gates/__init__.py b/src/forge/workflow/gates/__init__.py index 9cb10e290..5e481c865 100644 --- a/src/forge/workflow/gates/__init__.py +++ b/src/forge/workflow/gates/__init__.py @@ -6,6 +6,7 @@ from forge.workflow.gates.plan_approval import ( plan_approval_gate, + provision_epics, route_plan_approval, ) from forge.workflow.gates.prd_approval import ( @@ -17,6 +18,7 @@ spec_approval_gate, ) from forge.workflow.gates.task_approval import ( + provision_tasks, route_task_approval, task_approval_gate, ) @@ -32,8 +34,10 @@ "spec_approval_gate", "plan_approval_gate", "route_plan_approval", + "provision_epics", "route_task_approval", "task_approval_gate", + "provision_tasks", "route_task_plan_approval", "task_plan_approval_gate", ] diff --git a/src/forge/workflow/gates/plan_approval.py b/src/forge/workflow/gates/plan_approval.py index cd7acf3e2..20ae85c22 100644 --- a/src/forge/workflow/gates/plan_approval.py +++ b/src/forge/workflow/gates/plan_approval.py @@ -9,12 +9,16 @@ """ import logging +from typing import TYPE_CHECKING, Any, cast from langgraph.graph import END from forge.api.routes.metrics import record_approval, record_revision_requested from forge.workflow.feature.state import FeatureState as WorkflowState -from forge.workflow.utils import set_paused +from forge.workflow.utils import check_direct_mode, check_yolo_mode, set_paused + +if TYPE_CHECKING: + from forge.integrations.jira.client import JiraClient logger = logging.getLogger(__name__) @@ -38,24 +42,27 @@ def plan_approval_gate(state: WorkflowState) -> WorkflowState: epic_count = len(epic_keys) # Validate that we actually have epics to approve - if epic_count == 0: + if epic_count == 0 and (check_yolo_mode(state) or check_direct_mode(state)): logger.error( f"Plan approval gate reached with 0 Epics for {ticket_key}. " "This indicates epic decomposition failed. Routing back to retry." ) - return { - **state, - "last_error": "No Epics generated - decomposition may have failed", - "current_node": "decompose_epics", - "retry_count": state.get("retry_count", 0) + 1, - } + return cast( + WorkflowState, + { + **state, + "last_error": "No Epics generated - decomposition may have failed", + "current_node": "decompose_epics", + "retry_count": state.get("retry_count", 0) + 1, + }, + ) logger.info(f"Plan approval gate: pausing workflow for {ticket_key} ({epic_count} Epics)") - return set_paused(state, "plan_approval_gate") + return cast(WorkflowState, set_paused(cast(dict[str, Any], state), "plan_approval_gate")) -def route_plan_approval(state: WorkflowState) -> str: +async def route_plan_approval(state: WorkflowState) -> str: """Route based on plan approval status. Args: @@ -64,16 +71,19 @@ def route_plan_approval(state: WorkflowState) -> str: Returns: Next node name or END. """ + if state.get("current_node") == "decompose_epics": + return "decompose_epics" + # Check if this is a question (Q&A mode) - check FIRST if state.get("is_question") and state.get("feedback_comment"): logger.info(f"Q&A mode: routing to answer_question for {state['ticket_key']}") return "answer_question" # YOLO mode: auto-approve without human input - if state.get("yolo_mode"): + if check_yolo_mode(state): logger.info(f"YOLO mode: auto-approving plan for {state['ticket_key']}") record_approval("plan") - return "generate_tasks" + return "provision_epics" # Check if revision requested if state.get("revision_requested"): @@ -99,7 +109,111 @@ def route_plan_approval(state: WorkflowState) -> str: ) return END - # All Epics approved, proceed to task generation - logger.info(f"Epics approved for {state['ticket_key']}, proceeding to task generation") + # All Epics approved, proceed to standard epic provisioning node + logger.info(f"Epics approved for {state['ticket_key']}, proceeding to epic provisioning node") record_approval("plan") - return "generate_tasks" + return "provision_epics" + + +async def provision_epics(state: WorkflowState) -> WorkflowState: + """Standard LangGraph node to provision Epics from draft. + + Args: + state: Current workflow state. + + Returns: + Updated workflow state with epic_keys. + """ + ticket_key = state["ticket_key"] + if not state.get("epic_keys"): + from forge.integrations.jira.client import JiraClient + + jira = JiraClient() + try: + epic_keys = await provision_epics_from_draft(state, jira) + state = {**state, "epic_keys": epic_keys} + except Exception as e: + logger.error( + f"Failed ticket provisioning during plan approval for {ticket_key}: {e}", + exc_info=True, + ) + raise + finally: + await jira.close() + + return state + + +async def provision_epics_from_draft(state: WorkflowState, jira: "JiraClient") -> list[str]: + """Provision Epics from the plan draft attachment on Jira. + + Args: + state: The workflow state dictionary. + jira: An active JiraClient instance. + + Returns: + List of created Epic ticket keys. + """ + ticket_key = state["ticket_key"] + from forge.models.workflow import ForgeLabel + from forge.workflow.utils.draft_manager import FORGE_EPICS_DRAFT_FILENAME, DraftManager + + # Idempotency guard: check if Epics already exist on Jira with this parent label + jql = f'labels = "forge:parent:{ticket_key}" AND issuetype = Epic' + existing_issues = await jira.search_issues(jql) + if isinstance(existing_issues, list) and existing_issues: + existing_keys = [issue.key for issue in existing_issues] + logger.info( + f"Idempotency Guard: Found {len(existing_keys)} existing Epics for parent {ticket_key}: {existing_keys}. " + f"Skipping duplicate ticket creation, deleting draft and returning existing keys." + ) + try: + await DraftManager.delete_draft_attachment(jira, ticket_key, FORGE_EPICS_DRAFT_FILENAME) + except Exception as e: + logger.warning(f"Draft deletion skipped or failed during idempotency recovery: {e}") + return existing_keys + + logger.info(f"Retrieving plan draft for {ticket_key} from state") + draft_raw = state.get("plan_draft") + if not draft_raw: + raise ValueError( + f"Approved draft 'plan_draft' not found in workflow state for {ticket_key}" + ) + if isinstance(draft_raw, dict): + from forge.models.draft import ForgeDecompositionDraft + + draft = ForgeDecompositionDraft.model_validate(draft_raw) + else: + draft = draft_raw + + parent_issue = await jira.get_issue(ticket_key) + project_key = parent_issue.project_key + + epic_keys = [] + for item in draft.items: + if item.excluded: + logger.info(f"Skipping excluded plan item {item.id}: {item.summary}") + continue + + labels = [ + ForgeLabel.FORGE_MANAGED.value, + f"forge:parent:{ticket_key}", + ] + if item.repo and "/" in item.repo: + labels.append(f"repo:{item.repo}") + + epic_key = await jira.create_epic( + project_key=project_key, + summary=item.summary, + description=item.description, + parent_key=ticket_key, + labels=labels, + ) + epic_keys.append(epic_key) + + # Delete the draft only after 100% successful ticket creation + await DraftManager.delete_draft_attachment(jira, ticket_key, FORGE_EPICS_DRAFT_FILENAME) + logger.info( + f"Successfully provisioned {len(epic_keys)} Epics for {ticket_key} and deleted draft" + ) + return epic_keys diff --git a/src/forge/workflow/gates/task_approval.py b/src/forge/workflow/gates/task_approval.py index 32daceab0..c6c201568 100644 --- a/src/forge/workflow/gates/task_approval.py +++ b/src/forge/workflow/gates/task_approval.py @@ -9,12 +9,16 @@ """ import logging +from typing import TYPE_CHECKING, Any, cast from langgraph.graph import END from forge.api.routes.metrics import record_approval, record_revision_requested from forge.workflow.feature.state import FeatureState as WorkflowState -from forge.workflow.utils import set_paused +from forge.workflow.utils import check_direct_mode, check_yolo_mode, set_paused + +if TYPE_CHECKING: + from forge.integrations.jira.client import JiraClient logger = logging.getLogger(__name__) @@ -42,27 +46,30 @@ def task_approval_gate(state: WorkflowState) -> WorkflowState: task_count = len(task_keys) # Validate that we actually have tasks to approve - if task_count == 0: + if task_count == 0 and (check_yolo_mode(state) or check_direct_mode(state)): logger.error( f"Task approval gate reached with 0 Tasks for {ticket_key}. " "This indicates task generation failed. Routing back to retry." ) - return { - **state, - "last_error": "No Tasks generated - task generation may have failed", - "current_node": "generate_tasks", - "retry_count": state.get("retry_count", 0) + 1, - } + return cast( + WorkflowState, + { + **state, + "last_error": "No Tasks generated - task generation may have failed", + "current_node": "generate_tasks", + "retry_count": state.get("retry_count", 0) + 1, + }, + ) logger.info( f"Task approval gate: pausing workflow for {ticket_key} " f"({task_count} Tasks pending implementation approval)" ) - return set_paused(state, "task_approval_gate") + return cast(WorkflowState, set_paused(cast(dict[str, Any], state), "task_approval_gate")) -def route_task_approval(state: WorkflowState) -> str: +async def route_task_approval(state: WorkflowState) -> str: """Route based on task approval status. Routing logic: @@ -79,6 +86,9 @@ def route_task_approval(state: WorkflowState) -> str: Returns: Next node name or END. """ + if state.get("current_node") == "generate_tasks": + return "generate_tasks" + ticket_key = state["ticket_key"] # Check if this is a question (Q&A mode) - check FIRST @@ -87,10 +97,10 @@ def route_task_approval(state: WorkflowState) -> str: return "answer_question" # YOLO mode: auto-approve without human input - if state.get("yolo_mode"): + if check_yolo_mode(state): logger.info(f"YOLO mode: auto-approving tasks for {ticket_key}") record_approval("task") - return "task_router" + return "provision_tasks" # Check if revision requested (! feedback comment added) if state.get("revision_requested"): @@ -122,7 +132,167 @@ def route_task_approval(state: WorkflowState) -> str: ) return END - # Tasks approved, proceed to implementation - logger.info(f"Tasks approved for {ticket_key}, proceeding to implementation") + # Tasks approved, proceed to standard task provisioning node + logger.info(f"Tasks approved for {ticket_key}, proceeding to task provisioning node") record_approval("task") - return "task_router" + return "provision_tasks" + + +async def provision_tasks(state: WorkflowState) -> WorkflowState: + """Standard LangGraph node to provision Tasks from draft. + + Args: + state: Current workflow state. + + Returns: + Updated workflow state with task_keys and tasks_by_repo. + """ + ticket_key = state["ticket_key"] + if not state.get("task_keys"): + from forge.integrations.jira.client import JiraClient + + jira = JiraClient() + try: + task_keys, tasks_by_repo = await provision_tasks_from_draft(state, jira) + state = {**state, "task_keys": task_keys, "tasks_by_repo": tasks_by_repo} + except Exception as e: + logger.error( + f"Failed ticket provisioning during task approval for {ticket_key}: {e}", + exc_info=True, + ) + raise + finally: + await jira.close() + + return state + + +async def provision_tasks_from_draft( + state: WorkflowState, jira: "JiraClient" +) -> tuple[list[str], dict[str, list[str]]]: + """Provision Tasks from the task draft attachment on Jira. + + Args: + state: The workflow state dictionary. + jira: An active JiraClient instance. + + Returns: + Tuple of (task_keys, tasks_by_repo). + """ + ticket_key = state["ticket_key"] + from forge.config import get_settings + from forge.integrations.jira.client import MissingProjectConfig + from forge.models.workflow import ForgeLabel + + # Idempotency guard: check if Tasks already exist on Jira with this parent label + jql = f'labels = "forge:parent:{ticket_key}" AND issuetype = Task' + existing_issues = await jira.search_issues(jql) + if isinstance(existing_issues, list) and existing_issues: + existing_keys = [issue.key for issue in existing_issues] + logger.info( + f"Idempotency Guard: Found {len(existing_keys)} existing Tasks for parent {ticket_key}: {existing_keys}. " + "Skipping duplicate ticket creation and returning existing keys." + ) + + # Reconstruct tasks_by_repo from existing issues + reconstructed_tasks_by_repo: dict[str, list[str]] = {} + for issue in existing_issues: + repo = "unknown" + for label in issue.labels: + if label.startswith("repo:"): + repo = label[len("repo:") :] + break + + if repo not in reconstructed_tasks_by_repo: + reconstructed_tasks_by_repo[repo] = [] + reconstructed_tasks_by_repo[repo].append(issue.key) + + return existing_keys, reconstructed_tasks_by_repo + + settings = get_settings() + logger.info(f"Retrieving task draft for {ticket_key} from state") + draft_raw = state.get("tasks_draft") + if not draft_raw: + raise ValueError( + f"Approved draft 'tasks_draft' not found in workflow state for {ticket_key}" + ) + if isinstance(draft_raw, dict): + from forge.models.draft import ForgeDecompositionDraft + + draft = ForgeDecompositionDraft.model_validate(draft_raw) + else: + draft = draft_raw + + parent_issue = await jira.get_issue(ticket_key) + project_key = parent_issue.project_key + + task_keys: list[str] = [] + tasks_by_repo: dict[str, list[str]] = {} + for item in draft.items: + if item.excluded: + logger.info(f"Skipping excluded task item {item.id}: {item.summary}") + continue + + # Fallback repository logic (mimics task_generation.py) + repo = item.repo + if not repo or repo == "unknown" or "/" not in repo: + try: + repo = await jira.get_project_default_repo(project_key) + except MissingProjectConfig: + repo = ( + settings.github_default_repo + if not settings.forge_require_project_config + else "" + ) + + if not repo or "/" not in repo: + logger.warning( + f"Task '{item.summary}' has no valid repo. " + "Set repo labels on Feature/Epic or GITHUB_DEFAULT_REPO." + ) + repo = "unknown" + + # Epic parent key logic: + # If draft item has epic_key set, use it. + # Else fallback to state's epic_keys. + epic_key = item.epic_key + if not epic_key and state.get("epic_keys"): + epic_key = state["epic_keys"][0] + logger.warning( + f"Task '{item.summary}' (ID {item.id}) is missing epic_key. Reparenting to first Epic '{epic_key}'." + ) + try: + await jira.add_comment( + ticket_key, + f"⚠️ **Notice:** Task '{item.summary}' (ID: {item.id}) was missing an Epic parent key. " + f"It has been reparented to Epic '{epic_key}'.", + ) + except Exception as comment_err: + logger.warning(f"Failed to post reparenting notice comment: {comment_err}") + + # Labels + labels = [ + ForgeLabel.FORGE_MANAGED.value, + f"forge:parent:{ticket_key}", + ] + if repo and repo != "unknown": + labels.append(f"repo:{repo}") + + task_key = await jira.create_task( + project_key=project_key, + summary=item.summary, + description=item.description, + parent_key=epic_key, + labels=labels, + ) + task_keys.append(task_key) + + if repo and repo != "unknown": + if repo not in tasks_by_repo: + tasks_by_repo[repo] = [] + tasks_by_repo[repo].append(task_key) + + logger.info( + f"Successfully provisioned {len(task_keys)} Tasks across {len(tasks_by_repo)} repos" + ) + return task_keys, tasks_by_repo diff --git a/src/forge/workflow/nodes/epic_decomposition.py b/src/forge/workflow/nodes/epic_decomposition.py index 145650060..8ace0f343 100644 --- a/src/forge/workflow/nodes/epic_decomposition.py +++ b/src/forge/workflow/nodes/epic_decomposition.py @@ -1,14 +1,17 @@ """Epic decomposition node for LangGraph workflow.""" import logging -from typing import Any +from datetime import UTC, datetime +from typing import Any, cast from forge.config import get_settings from forge.integrations.agents import ForgeAgent from forge.integrations.jira.client import JiraClient, MissingProjectConfig +from forge.models.draft import DraftItem, ForgeDecompositionDraft from forge.models.workflow import ForgeLabel from forge.workflow.feature.state import FeatureState as WorkflowState -from forge.workflow.utils import update_state_timestamp +from forge.workflow.utils import check_direct_mode, check_yolo_mode, update_state_timestamp +from forge.workflow.utils.draft_manager import FORGE_EPICS_DRAFT_FILENAME, DraftManager from forge.workflow.utils.jira_status import post_status_comment from forge.workflow.utils.qa_summary import post_qa_summary_if_needed from forge.workflow.utils.references import fetch_and_inject_references @@ -88,18 +91,18 @@ async def decompose_epics(state: WorkflowState) -> WorkflowState: # 2. forge.repos Jira project property (required) feature_labels = await jira.get_labels(ticket_key) - available_repos = set() + available_repos_set: set[str] = set() # Add repos from Feature labels for label in feature_labels: if label.startswith("repo:"): - available_repos.add(label[5:]) + available_repos_set.add(label[5:]) # Add repos from Jira project property (required in strict mode) settings = get_settings() try: for repo in await get_effective_repos(jira, project_key): - available_repos.add(repo) + available_repos_set.add(repo) except MissingProjectConfig as e: if settings.forge_require_project_config: logger.error( @@ -122,7 +125,7 @@ async def decompose_epics(state: WorkflowState) -> WorkflowState: await jira.set_workflow_label(ticket_key, ForgeLabel.BLOCKED) return {**state, "last_error": str(e), "current_node": "decompose_epics"} - available_repos = list(available_repos) + available_repos: list[str] = list(available_repos_set) # Build context for Epic generation context: dict[str, Any] = { @@ -145,11 +148,15 @@ async def decompose_epics(state: WorkflowState) -> WorkflowState: if not epics_data: logger.warning(f"No Epics generated for {ticket_key}") - return { - **state, - "last_error": "Epic generation returned no results", - "current_node": "decompose_epics", - } + return cast( + WorkflowState, + { + **state, + "last_error": "Epic generation returned no results", + "current_node": "decompose_epics", + "retry_count": state.get("retry_count", 0) + 1, + }, + ) await ensure_repo_labels( jira, @@ -158,68 +165,190 @@ async def decompose_epics(state: WorkflowState) -> WorkflowState: [str(epic.get("repo", "")) for epic in epics_data], ) - # Create Epics in Jira - secondary operation - epics_by_repo: dict[str, list[str]] = {} - - for epic in epics_data: - summary = epic.get("summary", "Untitled Epic") - plan = epic.get("plan", "") - repo = epic.get("repo", "") - - # Build labels for the Epic - # Include forge:managed for webhook routing and forge:parent for lookup - labels = [ - ForgeLabel.FORGE_MANAGED.value, - f"forge:parent:{ticket_key}", - *workflow_identity_labels(state), - ] - if repo and "/" in repo: - labels.append(f"repo:{repo}") - # Track which epics go to which repo - if repo not in epics_by_repo: - epics_by_repo[repo] = [] + # Check parent Jira ticket labels to check for forge:yolo and inspect global config yolo_mode + is_yolo = check_yolo_mode(state, feature_labels) + is_direct = check_direct_mode(state, feature_labels) - try: - epic_key = await jira.create_epic( - project_key=project_key, - summary=summary, - description=plan, - parent_key=ticket_key, - labels=labels, + if is_yolo or is_direct: + # Create Epics in Jira immediately + epics_by_repo: dict[str, list[str]] = {} + + for epic in epics_data: + summary = epic.get("summary", "Untitled Epic") + plan = epic.get("plan", "") + repo = epic.get("repo", "") + + # Build labels for the Epic + # Include forge:managed for webhook routing and forge:parent for lookup + labels = [ + ForgeLabel.FORGE_MANAGED.value, + f"forge:parent:{ticket_key}", + *workflow_identity_labels(state), + ] + if repo and "/" in repo: + labels.append(f"repo:{repo}") + # Track which epics go to which repo + if repo not in epics_by_repo: + epics_by_repo[repo] = [] + + try: + epic_key = await jira.create_epic( + project_key=project_key, + summary=summary, + description=plan, + parent_key=ticket_key, + labels=labels, + ) + epic_keys.append(epic_key) + + if repo: + epics_by_repo[repo].append(epic_key) + + logger.info( + f"Created Epic {epic_key}: {summary}" + (f" (repo: {repo})" if repo else "") + ) + except Exception as e: + # Log but continue creating remaining Epics + jira_error = str(e) + logger.warning(f"Failed to create Epic '{summary}' for {ticket_key}: {e}") + + logger.info(f"Created {len(epic_keys)} Epics for {ticket_key}") + + # If we created some Epics, advance even with partial failures + if epic_keys: + # Only set workflow label after confirming epics were created + try: + await jira.set_workflow_label(ticket_key, ForgeLabel.PLAN_PENDING) + except Exception as e: + jira_error = str(e) + logger.warning(f"Failed to set workflow label for {ticket_key}: {e}") + + await jira.add_comment( + ticket_key, + "## 🤖 Forge interaction options\n\n" + f"- ✅ **Approve:** add `{ForgeLabel.PLAN_APPROVED.value}` to continue.\n" + "- ♻️ **Revise all epics:** add a comment starting with `!` on this ticket.\n" + "- 🔧 **Revise a single epic:** add a comment starting with `!` on the Epic.\n" + "- ❓ **Ask a question:** add a Jira comment starting with `?`.\n\n" + "### Supported Workflow Modes\n" + "1. **Default Draft Review Flow:** Forge attaches a draft JSON and posts a detailed markdown preview. Users can use `/forge` commands or comment starting with `!` to revise, and approve via `/forge approve` or adding the `forge:plan-approved` label.\n" + "2. **Direct Mode (`forge:direct-mode`):** Forge bypasses draft attachments and directly creates the Epic issues in Jira immediately, then pauses awaiting human approval (adding `forge:plan-approved` label).\n" + "3. **YOLO Mode (`forge:yolo`):** Forge bypasses draft attachments and human approval gates, automatically creating the Epic issues in Jira and auto-advancing without pausing.", ) - epic_keys.append(epic_key) - if repo: - epics_by_repo[repo].append(epic_key) + # Store plan summary in generation_context so Q&A can reference it + generation_context = state.get("generation_context", {}) + plan_summary_parts = [] + for epic in epics_data: + summary = epic.get("summary", "") + plan = epic.get("plan", "") + repo = epic.get("repo", "") + plan_summary_parts.append( + f"## {summary}" + (f" (repo: {repo})" if repo else "") + f"\n{plan}" + ) + generation_context["plan"] = "\n\n".join(plan_summary_parts) + + return cast( + WorkflowState, + update_state_timestamp( + { + **state, + "epic_keys": epic_keys, + "generation_context": generation_context, + "feedback_comment": None, + "revision_requested": False, + "current_epic_key": None, + "current_node": "plan_approval_gate", + "is_paused": not is_yolo, + "last_error": f"Partial Jira failure: {jira_error}" + if jira_error + else None, + } + ), + ) + else: + # No Epics created at all - this is a failure + return cast( + WorkflowState, + { + **state, + "last_error": jira_error or "Failed to create any Epics in Jira", + "current_node": "decompose_epics", + "retry_count": state.get("retry_count", 0) + 1, + }, + ) + else: + # Draft Review Flow (YOLO is inactive) + # Empty-draft guard to prevent proceeding without draft epics + if not epics_data: + return cast( + WorkflowState, + { + **state, + "last_error": "Failed to generate any draft Epics", + "current_node": "decompose_epics", + "retry_count": state.get("retry_count", 0) + 1, + }, + ) - logger.info( - f"Created Epic {epic_key}: {summary}" + (f" (repo: {repo})" if repo else "") + # Prior to saving, check for existing forge-epics-draft.json attachments + # and delete them using DraftManager/JiraClient to prevent duplicate file accumulation. + try: + await DraftManager.delete_draft_attachment( + jira, ticket_key, FORGE_EPICS_DRAFT_FILENAME ) except Exception as e: - # Log but continue creating remaining Epics - jira_error = str(e) - logger.warning(f"Failed to create Epic '{summary}' for {ticket_key}: {e}") + logger.warning(f"Failed to delete existing draft attachment: {e}") - logger.info(f"Created {len(epic_keys)} Epics for {ticket_key}") + # Convert epics_data into DraftItem instances + draft_items = [] + for idx, epic in enumerate(epics_data, start=1): + summary = epic.get("summary", "Untitled Epic") + plan = epic.get("plan", "") + repo = epic.get("repo", "") + draft_items.append( + DraftItem( + id=idx, + summary=summary, + description=plan, + repo=repo, + acceptance_criteria=[], + excluded=False, + ) + ) - # If we created some Epics, advance even with partial failures - if epic_keys: - # Only set workflow label after confirming epics were created + # Create Draft model + draft = ForgeDecompositionDraft( + parent_key=ticket_key, + phase="epics", + items=draft_items, + version=1, + created_at=datetime.now(UTC), + updated_at=datetime.now(UTC), + ) + + # Format Markdown review comment outlining proposed items + # Implement BR-003 Truncation Boundary + comment_body = DraftManager.format_review_comment(draft) + + # Post the review comment to the parent Jira ticket + await jira.add_comment(ticket_key, comment_body) + + # Save draft as attachment + await DraftManager.save_draft_attachment( + jira, + ticket_key, + draft, + FORGE_EPICS_DRAFT_FILENAME, + ) + + # Set workflow label to pending try: await jira.set_workflow_label(ticket_key, ForgeLabel.PLAN_PENDING) except Exception as e: jira_error = str(e) logger.warning(f"Failed to set workflow label for {ticket_key}: {e}") - await jira.add_comment( - ticket_key, - "## 🤖 Forge interaction options\n\n" - f"- ✅ **Approve:** add `{ForgeLabel.PLAN_APPROVED.value}` to continue.\n" - "- ♻️ **Revise all epics:** add a comment starting with `!` on this ticket.\n" - "- 🔧 **Revise a single epic:** add a comment starting with `!` on the Epic.\n" - "- ❓ **Ask a question:** add a Jira comment starting with `?`.", - ) - # Store plan summary in generation_context so Q&A can reference it generation_context = state.get("generation_context", {}) plan_summary_parts = [] @@ -232,26 +361,24 @@ async def decompose_epics(state: WorkflowState) -> WorkflowState: ) generation_context["plan"] = "\n\n".join(plan_summary_parts) - return update_state_timestamp( - { - **state, - "epic_keys": epic_keys, - "generation_context": generation_context, - "feedback_comment": None, - "revision_requested": False, - "current_epic_key": None, - "current_node": "plan_approval_gate", - "last_error": f"Partial Jira failure: {jira_error}" if jira_error else None, - } + # Transition state to pause the workflow at the plan_approval_gate (setting is_paused = True and appropriate workflow flags) + return cast( + WorkflowState, + update_state_timestamp( + { + **state, + "plan_draft": draft, + "epic_keys": [], + "generation_context": generation_context, + "feedback_comment": None, + "revision_requested": False, + "current_epic_key": None, + "current_node": "plan_approval_gate", + "is_paused": True, + "last_error": f"Partial Jira failure: {jira_error}" if jira_error else None, + } + ), ) - else: - # No Epics created at all - this is a failure - return { - **state, - "last_error": jira_error or "Failed to create any Epics in Jira", - "current_node": "decompose_epics", - "retry_count": state.get("retry_count", 0) + 1, - } except Exception as e: logger.error(f"Epic decomposition failed for {ticket_key}: {e}") @@ -264,7 +391,7 @@ async def decompose_epics(state: WorkflowState) -> WorkflowState: } if epic_keys: result_state["epic_keys"] = epic_keys - return result_state + return cast(WorkflowState, result_state) finally: await jira.close() await agent.close() @@ -307,16 +434,19 @@ async def regenerate_all_epics(state: WorkflowState) -> WorkflowState: } # Re-run decomposition (which will use context including feedback) - return await decompose_epics(updated_state) + return await decompose_epics(cast(WorkflowState, updated_state)) except Exception as e: logger.error(f"Epic regeneration failed for {ticket_key}: {e}") - return { - **state, - "last_error": str(e), - "current_node": "regenerate_all_epics", - "retry_count": state.get("retry_count", 0) + 1, - } + return cast( + WorkflowState, + { + **state, + "last_error": str(e), + "current_node": "regenerate_all_epics", + "retry_count": state.get("retry_count", 0) + 1, + }, + ) finally: await jira.close() @@ -334,7 +464,7 @@ async def update_single_epic(state: WorkflowState) -> WorkflowState: """ ticket_key = state["ticket_key"] epic_key = state.get("current_epic_key") - feedback = state.get("feedback_comment", "") + feedback = state.get("feedback_comment") or "" if not epic_key: logger.warning(f"No current_epic_key for single Epic update on {ticket_key}") @@ -379,25 +509,31 @@ async def update_single_epic(state: WorkflowState) -> WorkflowState: logger.info(f"Updated Epic {epic_key} plan") - return update_state_timestamp( - { - **state, - "current_epic_key": None, - "feedback_comment": None, - "revision_requested": False, - "current_node": "plan_approval_gate", - "last_error": None, - } + return cast( + WorkflowState, + update_state_timestamp( + { + **state, + "current_epic_key": None, + "feedback_comment": None, + "revision_requested": False, + "current_node": "plan_approval_gate", + "last_error": None, + } + ), ) except Exception as e: logger.error(f"Epic update failed for {epic_key}: {e}") - return { - **state, - "last_error": str(e), - "current_node": "update_single_epic", - "retry_count": state.get("retry_count", 0) + 1, - } + return cast( + WorkflowState, + { + **state, + "last_error": str(e), + "current_node": "update_single_epic", + "retry_count": state.get("retry_count", 0) + 1, + }, + ) finally: await jira.close() await agent.close() diff --git a/src/forge/workflow/nodes/task_generation.py b/src/forge/workflow/nodes/task_generation.py index c1f36de45..ba4b05cd5 100644 --- a/src/forge/workflow/nodes/task_generation.py +++ b/src/forge/workflow/nodes/task_generation.py @@ -3,14 +3,18 @@ import asyncio import logging import re -from typing import Any +from datetime import UTC, datetime +from typing import Any, cast +from forge.config import get_settings # noqa: F401 from forge.integrations.agents import ForgeAgent from forge.integrations.jira.client import JiraClient, MissingProjectConfig +from forge.models.draft import DraftItem, ForgeDecompositionDraft from forge.models.workflow import ForgeLabel from forge.prompts import load_prompt from forge.workflow.feature.state import FeatureState as WorkflowState -from forge.workflow.utils import update_state_timestamp +from forge.workflow.utils import check_direct_mode, check_yolo_mode, update_state_timestamp +from forge.workflow.utils.draft_manager import DraftManager from forge.workflow.utils.jira_status import post_status_comment from forge.workflow.utils.references import fetch_and_inject_references from forge.workflow.utils.repo_resolution import get_effective_default_repo @@ -72,6 +76,10 @@ async def generate_tasks(state: WorkflowState) -> WorkflowState: # Get project key from parent Feature parent_issue = await jira.get_issue(ticket_key) project_key = parent_issue.project_key + feature_labels = await jira.get_labels(ticket_key) + + is_yolo = check_yolo_mode(state, feature_labels) + is_direct = check_direct_mode(state, feature_labels) # Pre-fetch all epic details upfront for sibling context for ek in epic_keys: @@ -88,6 +96,8 @@ async def generate_tasks(state: WorkflowState) -> WorkflowState: logger.warning(f"Failed to pre-fetch Epic {ek}: {e}") all_epics_details.append({"epic_key": ek, "epic_summary": ek, "epic_plan": ""}) + proposed_tasks_list = [] + for epic_key in epic_keys: logger.info(f"Generating Tasks for Epic {epic_key}") @@ -138,7 +148,7 @@ async def generate_tasks(state: WorkflowState) -> WorkflowState: existing_tasks=created_tasks_context if created_tasks_context else None, ) - # Create Tasks in Jira - secondary operation + # Create Tasks in Jira (YOLO) or collect (non-YOLO) for task in tasks_data: summary = task.get("summary", "Untitled Task") description = task.get("description", "") @@ -163,89 +173,195 @@ async def generate_tasks(state: WorkflowState) -> WorkflowState: ) repo = "unknown" - # Add labels: forge:managed for webhook routing, forge:parent for lookup, repo - labels = [ - ForgeLabel.FORGE_MANAGED.value, - f"forge:parent:{ticket_key}", # Parent Feature key - *workflow_identity_labels(state), - ] - if repo and repo != "unknown": - labels.append(f"repo:{repo}") + if is_yolo or is_direct: + # Add labels: forge:managed for webhook routing, forge:parent for lookup, repo + labels = [ + ForgeLabel.FORGE_MANAGED.value, + f"forge:parent:{ticket_key}", # Parent Feature key + *workflow_identity_labels(state), + ] + if repo and repo != "unknown": + labels.append(f"repo:{repo}") - try: - task_key = await jira.create_task( - project_key=project_key, - summary=summary, - description=description, - parent_key=epic_key, - labels=labels, + try: + task_key = await jira.create_task( + project_key=project_key, + summary=summary, + description=description, + parent_key=epic_key, + labels=labels, + ) + + all_task_keys.append(task_key) + + # Track by repository + if repo not in tasks_by_repo: + tasks_by_repo[repo] = [] + tasks_by_repo[repo].append(task_key) + + # Track for context in subsequent epic task generation + created_tasks_context.append( + { + "epic_key": epic_key, + "epic_summary": epic_summary, + "task_key": task_key, + "summary": summary, + } + ) + + logger.info(f"Created Task {task_key}: {summary} (repo: {repo})") + except Exception as e: + # Log but continue creating remaining Tasks + jira_error = str(e) + logger.warning(f"Failed to create Task '{summary}' for {ticket_key}: {e}") + else: + # Non-YOLO mode: collect proposed task details for draft + proposed_tasks_list.append( + { + "summary": summary, + "description": description, + "repo": repo, + "epic_key": epic_key, + } ) - - all_task_keys.append(task_key) - - # Track by repository - if repo not in tasks_by_repo: - tasks_by_repo[repo] = [] - tasks_by_repo[repo].append(task_key) - - # Track for context in subsequent epic task generation + # Track for context in sibling generations + virtual_key = f"Draft Task {len(proposed_tasks_list)}" created_tasks_context.append( { "epic_key": epic_key, "epic_summary": epic_summary, - "task_key": task_key, + "task_key": virtual_key, "summary": summary, } ) - logger.info(f"Created Task {task_key}: {summary} (repo: {repo})") + if is_yolo or is_direct: + logger.info( + f"Created {len(all_task_keys)} Tasks for {ticket_key}, awaiting implementation approval" + ) + + # If we created some Tasks, advance even with partial failures + if all_task_keys: + # Only set workflow label after confirming tasks were created + try: + await jira.set_workflow_label(ticket_key, ForgeLabel.TASK_PENDING) except Exception as e: - # Log but continue creating remaining Tasks jira_error = str(e) - logger.warning(f"Failed to create Task '{summary}' for {ticket_key}: {e}") + logger.warning(f"Failed to set workflow label for {ticket_key}: {e}") + + await jira.add_comment( + ticket_key, + "## 🤖 Forge interaction options\n\n" + f"- ✅ **Approve:** add `{ForgeLabel.TASK_APPROVED.value}` to continue.\n" + "- ♻️ **Revise all tasks:** add a comment starting with `!` on this ticket.\n" + "- 🔧 **Revise a single task:** add a comment starting with `!` on the Task.\n" + "- ❓ **Ask a question:** add a Jira comment starting with `?`.\n\n" + "### Supported Workflow Modes\n" + "1. **Default Draft Review Flow:** Forge attaches a draft JSON and posts a detailed markdown preview. Users can use `/forge` commands or comment starting with `!` to revise, and approve via `/forge approve` or adding the `forge:task-approved` label.\n" + "2. **Direct Mode (`forge:direct-mode`):** Forge bypasses draft attachments and directly creates the Task issues in Jira immediately, then pauses awaiting human approval (adding `forge:task-approved` label).\n" + "3. **YOLO Mode (`forge:yolo`):** Forge bypasses draft attachments and human approval gates, automatically creating the Task issues in Jira and auto-advancing without pausing.", + ) + return cast( + WorkflowState, + update_state_timestamp( + { + **state, + "task_keys": all_task_keys, + "tasks_by_repo": tasks_by_repo, + "feedback_comment": None, + "revision_requested": False, + "current_task_key": None, + "current_epic_key": None, + "current_node": "task_approval_gate", + "is_paused": not is_yolo, + "last_error": f"Partial Jira failure: {jira_error}" + if jira_error + else None, + } + ), + ) + else: + # No Tasks created at all - this is a failure + return cast( + WorkflowState, + { + **state, + "last_error": jira_error or "Failed to create any Tasks in Jira", + "current_node": "generate_tasks", + "retry_count": state.get("retry_count", 0) + 1, + }, + ) + else: + # Non-YOLO mode: Draft Review Flow + if not proposed_tasks_list: + return cast( + WorkflowState, + { + **state, + "last_error": "Failed to generate any draft Tasks", + "current_node": "generate_tasks", + "retry_count": state.get("retry_count", 0) + 1, + }, + ) - logger.info( - f"Created {len(all_task_keys)} Tasks for {ticket_key}, awaiting implementation approval" - ) + # Convert proposed_tasks_list into DraftItem instances + draft_items = [] + for idx, task_item in enumerate(proposed_tasks_list, start=1): + summary = task_item.get("summary", "Untitled Task") + description = task_item.get("description", "") + repo = task_item.get("repo", "unknown") + item_epic_key = task_item.get("epic_key") + draft_items.append( + DraftItem( + id=idx, + summary=summary, + description=description, + repo=repo, + epic_key=item_epic_key, + acceptance_criteria=[], + excluded=False, + ) + ) - # If we created some Tasks, advance even with partial failures - if all_task_keys: - # Only set workflow label after confirming tasks were created + # Create Draft model + draft = ForgeDecompositionDraft( + parent_key=ticket_key, + phase="tasks", + items=draft_items, + version=1, + created_at=datetime.now(UTC), + updated_at=datetime.now(UTC), + ) + + # Post task draft review comments to Epic tickets and a navigation comment on Feature + await DraftManager.post_task_draft_review(jira, ticket_key, draft) + + # Set workflow label to pending try: await jira.set_workflow_label(ticket_key, ForgeLabel.TASK_PENDING) except Exception as e: jira_error = str(e) logger.warning(f"Failed to set workflow label for {ticket_key}: {e}") - await jira.add_comment( - ticket_key, - "## 🤖 Forge interaction options\n\n" - f"- ✅ **Approve:** add `{ForgeLabel.TASK_APPROVED.value}` to continue.\n" - "- ♻️ **Revise all tasks:** add a comment starting with `!` on this ticket.\n" - "- 🔧 **Revise a single task:** add a comment starting with `!` on the Task.\n" - "- ❓ **Ask a question:** add a Jira comment starting with `?`.", - ) - return update_state_timestamp( - { - **state, - "task_keys": all_task_keys, - "tasks_by_repo": tasks_by_repo, - "feedback_comment": None, - "revision_requested": False, - "current_task_key": None, - "current_epic_key": None, - "current_node": "task_approval_gate", - "last_error": (f"Partial Jira failure: {jira_error}" if jira_error else None), - } + # Transition state to pause the workflow at the task_approval_gate (setting is_paused = True and appropriate workflow flags) + return cast( + WorkflowState, + update_state_timestamp( + { + **state, + "tasks_draft": draft, + "task_keys": [], + "tasks_by_repo": {}, + "feedback_comment": None, + "revision_requested": False, + "current_task_key": None, + "current_epic_key": None, + "current_node": "task_approval_gate", + "is_paused": True, + "last_error": f"Partial Jira failure: {jira_error}" if jira_error else None, + } + ), ) - else: - # No Tasks created at all - this is a failure - return { - **state, - "last_error": jira_error or "Failed to create any Tasks in Jira", - "current_node": "generate_tasks", - "retry_count": state.get("retry_count", 0) + 1, - } except Exception as e: logger.error(f"Task generation failed for {ticket_key}: {e}") @@ -259,9 +375,10 @@ async def generate_tasks(state: WorkflowState) -> WorkflowState: if all_task_keys: result_state["task_keys"] = all_task_keys result_state["tasks_by_repo"] = tasks_by_repo - return result_state + return cast(WorkflowState, result_state) finally: await jira.close() + await agent.close() async def _generate_tasks_for_epic( @@ -496,16 +613,19 @@ async def regenerate_all_tasks(state: WorkflowState) -> WorkflowState: } # Re-run task generation (which will incorporate feedback in context) - return await generate_tasks(updated_state) + return await generate_tasks(cast(WorkflowState, updated_state)) except Exception as e: logger.error(f"Task regeneration failed for {ticket_key}: {e}") - return { - **state, - "last_error": str(e), - "current_node": "regenerate_all_tasks", - "retry_count": state.get("retry_count", 0) + 1, - } + return cast( + WorkflowState, + { + **state, + "last_error": str(e), + "current_node": "regenerate_all_tasks", + "retry_count": state.get("retry_count", 0) + 1, + }, + ) finally: await jira.close() @@ -750,31 +870,37 @@ async def _fetch_sibling(ek: str) -> dict[str, str] | None: all_task_keys = remaining_task_keys + new_task_keys logger.info(f"Regenerated {len(new_task_keys)} tasks for Epic {epic_key} on {ticket_key}") - return update_state_timestamp( + return cast( + WorkflowState, + update_state_timestamp( + { + **state, + "task_keys": all_task_keys, + "tasks_by_repo": remaining_tasks_by_repo, + "feedback_comment": None, + "revision_requested": False, + "current_epic_key": None, + "current_node": "task_approval_gate", + "last_error": f"Partial Jira failure: {jira_error}" if jira_error else None, + } + ), + ) + + except Exception as e: + logger.error(f"Epic task regeneration failed for {epic_key} on {ticket_key}: {e}") + return cast( + WorkflowState, { **state, - "task_keys": all_task_keys, - "tasks_by_repo": remaining_tasks_by_repo, - "feedback_comment": None, + "last_error": str(e), + "current_node": "regenerate_epic_tasks", + "retry_count": state.get("retry_count", 0) + 1, + # Clear revision flags so task_approval_gate returns END instead of looping "revision_requested": False, + "feedback_comment": None, "current_epic_key": None, - "current_node": "task_approval_gate", - "last_error": (f"Partial Jira failure: {jira_error}" if jira_error else None), - } + }, ) - - except Exception as e: - logger.error(f"Epic task regeneration failed for {epic_key} on {ticket_key}: {e}") - return { - **state, - "last_error": str(e), - "current_node": "regenerate_epic_tasks", - "retry_count": state.get("retry_count", 0) + 1, - # Clear revision flags so task_approval_gate returns END instead of looping - "revision_requested": False, - "feedback_comment": None, - "current_epic_key": None, - } finally: await jira.close() await agent.close() @@ -793,7 +919,7 @@ async def update_single_task(state: WorkflowState) -> WorkflowState: """ ticket_key = state["ticket_key"] task_key = state.get("current_task_key") - feedback = state.get("feedback_comment", "") + feedback = state.get("feedback_comment") or "" if not task_key: logger.warning(f"No current_task_key for single Task update on {ticket_key}") @@ -840,25 +966,31 @@ async def update_single_task(state: WorkflowState) -> WorkflowState: logger.info(f"Task {task_key} updated with feedback") - return update_state_timestamp( - { - **state, - "current_task_key": None, - "feedback_comment": None, - "revision_requested": False, - "current_node": "task_approval_gate", - "last_error": None, - } + return cast( + WorkflowState, + update_state_timestamp( + { + **state, + "current_task_key": None, + "feedback_comment": None, + "revision_requested": False, + "current_node": "task_approval_gate", + "last_error": None, + } + ), ) except Exception as e: logger.error(f"Task update failed for {task_key}: {e}") - return { - **state, - "last_error": str(e), - "current_node": "update_single_task", - "retry_count": state.get("retry_count", 0) + 1, - } + return cast( + WorkflowState, + { + **state, + "last_error": str(e), + "current_node": "update_single_task", + "retry_count": state.get("retry_count", 0) + 1, + }, + ) finally: await jira.close() await agent.close() diff --git a/src/forge/workflow/utils/__init__.py b/src/forge/workflow/utils/__init__.py index 426e09093..df7b58905 100644 --- a/src/forge/workflow/utils/__init__.py +++ b/src/forge/workflow/utils/__init__.py @@ -5,7 +5,16 @@ from langgraph.graph import END -from forge.workflow.utils.comment_classifier import CommentType, classify_comment +from forge.workflow.utils.comment_classifier import ( + CommentType, + classify_comment, + parse_comment_command, +) +from forge.workflow.utils.draft_manager import ( + FORGE_EPICS_DRAFT_FILENAME, + FORGE_TASKS_DRAFT_FILENAME, + DraftManager, +) from forge.workflow.utils.jira_status import ( post_status_comment, remove_implementing_label, @@ -89,11 +98,50 @@ def set_error(state: dict[str, Any], error: str) -> dict[str, Any]: } +def check_yolo_mode(state: Any, labels: list[str] | None = None) -> bool: + """Check if YOLO mode is enabled based on labels or state. + + The components are: + 1. 'forge:yolo' label in the provided labels or the state context labels. + 2. State yolo_mode. + """ + # 1. Label check + has_label = False + if (labels and "forge:yolo" in labels) or "forge:yolo" in state.get("context", {}).get( + "labels", [] + ): + has_label = True + + # 2. State check + return has_label or bool(state.get("yolo_mode", False)) + + +def check_direct_mode(state: Any, labels: list[str] | None = None) -> bool: + """Check if direct ticket creation mode is enabled based on labels or state. + + The components are: + 1. ForgeLabel.DIRECT_MODE label in the provided labels or the state context labels. + 2. State direct_mode. + """ + from forge.models.workflow import ForgeLabel + + has_label = False + if (labels and ForgeLabel.DIRECT_MODE in labels) or ForgeLabel.DIRECT_MODE in state.get( + "context", {} + ).get("labels", []): + has_label = True + + return has_label or bool(state.get("direct_mode", False)) + + __all__ = [ + "check_yolo_mode", + "check_direct_mode", "CommentType", "classify_comment", "collect_review_exhaustion", "merge_review_exhaustion", + "parse_comment_command", "post_qa_summary_if_needed", "post_status_comment", "remove_implementing_label", @@ -106,4 +154,7 @@ def set_error(state: dict[str, Any], error: str) -> dict[str, Any]: "set_review_pending_label", "transition_tasks_to_in_progress", "update_state_timestamp", + "FORGE_EPICS_DRAFT_FILENAME", + "FORGE_TASKS_DRAFT_FILENAME", + "DraftManager", ] diff --git a/src/forge/workflow/utils/comment_classifier.py b/src/forge/workflow/utils/comment_classifier.py index 8caf9b5eb..a8b97a776 100644 --- a/src/forge/workflow/utils/comment_classifier.py +++ b/src/forge/workflow/utils/comment_classifier.py @@ -2,6 +2,7 @@ import re from enum import StrEnum +from typing import Any class CommentType(StrEnum): @@ -10,6 +11,7 @@ class CommentType(StrEnum): QUESTION = "question" FEEDBACK = "feedback" INFORMATIONAL = "informational" + COMMAND = "command" # Legacy @forge ask pattern (case insensitive). @@ -21,12 +23,143 @@ class CommentType(StrEnum): # Pattern for revision prefix (allowing leading whitespace) _REVISION_PATTERN = re.compile(r"^\s*!") +# Regex to match case-insensitive /forge command prefix followed by command name +_FORGE_COMMAND_PATTERN = re.compile(r"^\s*/forge\s+([a-zA-Z0-9_-]+)", re.IGNORECASE) + +# Regex to match key-value pairs supporting single/double quoted string values or unquoted values +_SINGLE_PAIR_PATTERN = re.compile( + r'\s*([a-zA-Z_][a-zA-Z0-9_-]*)\s*=\s*(?:"([^"]*)"|\'([^\']*)\'|([^\s\'"]+))' +) + + +def _parse_key_values(args_text: str) -> dict[str, str]: + """Parse key-value pairs from argument text. + + Args: + args_text: The text to parse. + + Returns: + A dictionary of parsed parameters. + + Raises: + ValueError: If parameters are malformed. + """ + pos = 0 + params = {} + while pos < len(args_text): + m = _SINGLE_PAIR_PATTERN.match(args_text, pos) + if not m: + raise ValueError(f"Malformed parameters or trailing junk near: '{args_text[pos:]}'") + key = m.group(1) + val = ( + m.group(2) + if m.group(2) is not None + else (m.group(3) if m.group(3) is not None else m.group(4)) + ) + params[key] = val + pos = m.end() + return params + + +def parse_comment_command(comment_text: str) -> dict[str, Any] | None: + """Parse a /forge comment command and extract its parameters. + + Supported commands: + - remove: /forge remove + - add: /forge add key=val key2="val with spaces" + - update: /forge update key=val key2="val with spaces" + - exclude: /forge exclude + + Args: + comment_text: The comment text to parse. + + Returns: + A dictionary containing the parsed 'command' and arguments, + or an 'error' description if parameters are malformed, + or None if not a recognized /forge command. + """ + if not comment_text or not comment_text.strip(): + return None + + match = _FORGE_COMMAND_PATTERN.match(comment_text) + if not match: + return None + + cmd_name = match.group(1).lower() + valid_commands = {"remove", "add", "update", "exclude"} + if cmd_name not in valid_commands: + return None + + args_text = comment_text[match.end() :].strip() + + if cmd_name in ("remove", "exclude"): + if not args_text: + return { + "command": cmd_name, + "error": f"Missing integer ID for {cmd_name} command", + } + if re.match(r"^\d+$", args_text): + return {"command": cmd_name, "id": int(args_text)} + return { + "command": cmd_name, + "error": f"Invalid integer ID for {cmd_name} command: '{args_text}'", + } + + if cmd_name == "add": + if not args_text: + return { + "command": "add", + "error": "Missing key-value parameters for add command", + } + try: + params = _parse_key_values(args_text) + except ValueError as e: + return { + "command": "add", + "error": str(e), + } + return {"command": "add", "params": params} + + if cmd_name == "update": + if not args_text: + return { + "command": "update", + "error": "Missing integer ID and parameters for update command", + } + id_match = re.match(r"^(\d+)(?:\s+(.*))?$", args_text) + if not id_match: + first_word = args_text.split(None, 1)[0] + if not re.match(r"^\d+$", first_word): + return { + "command": "update", + "error": f"Invalid integer ID for update command: '{first_word}'", + } + return { + "command": "update", + "error": "Missing integer ID for update command", + } + id_val = int(id_match.group(1)) + params_text = (id_match.group(2) or "").strip() + params = {} + if params_text: + try: + params = _parse_key_values(params_text) + except ValueError as e: + return { + "command": "update", + "error": str(e), + } + return {"command": "update", "id": id_val, "params": params} + + return None + def classify_comment(comment_text: str) -> CommentType: - """Classify a comment into question, feedback, or informational. + """Classify a comment into question, feedback, command, or informational. Classification rules: - - Questions: Comments starting with '?' + - Commands: Comments starting with /forge (except skip-gate/unskip-gate) + - Questions: Comments starting with '?' or '@forge ask' - Feedback (revision request): Comments starting with '!' - Informational: Everything else — ignored by the workflow @@ -42,6 +175,11 @@ def classify_comment(comment_text: str) -> CommentType: if not comment_text or not comment_text.strip(): return CommentType.INFORMATIONAL + # Check for commands first, since they are specific prefix patterns. + # Note: skip-gate/unskip-gate are excluded and should not return CommentType.COMMAND. + if parse_comment_command(comment_text) is not None: + return CommentType.COMMAND + if _QUESTION_MARK_PATTERN.match(comment_text): return CommentType.QUESTION diff --git a/src/forge/workflow/utils/draft_manager.py b/src/forge/workflow/utils/draft_manager.py new file mode 100644 index 000000000..02a844ad0 --- /dev/null +++ b/src/forge/workflow/utils/draft_manager.py @@ -0,0 +1,488 @@ +"""Utility for managing draft CRUD operations on Jira parent tickets as attachments.""" + +import copy +import logging +from datetime import UTC, datetime +from typing import Any + +from pydantic import ValidationError + +from forge.integrations.jira import JiraClient +from forge.models.draft import DraftItem, ForgeDecompositionDraft + +logger = logging.getLogger(__name__) + +FORGE_EPICS_DRAFT_FILENAME = "forge-epics-draft.json" +FORGE_TASKS_DRAFT_FILENAME = "forge-tasks-draft.json" + + +class DraftManager: + """Manages draft CRUD operations on Jira parent tickets as attachments.""" + + @staticmethod + def _validate_item_params( + params: dict[str, Any], target_item: dict[str, Any] | None = None + ) -> None: + """Validate the fields in draft item parameters strictly. + + Args: + params: The parameters dictionary. + target_item: Optional target item dictionary to merge with (for update command). + + Raises: + ValueError: If a validation check fails. + """ + from forge.models.draft import DraftItem + + if target_item is not None: + full_item = {**target_item, **params} + else: + defaults = { + "id": 1, + "summary": "", + "description": "", + "repo": "", + "acceptance_criteria": [], + "excluded": False, + "epic_key": None, + } + full_item = {**defaults, **params} + + try: + DraftItem.model_validate(full_item, strict=True) + except ValidationError as e: + for error in e.errors(): + loc = error["loc"] + if not loc: + continue + field = str(loc[0]) + error_type = error["type"] + if error_type == "extra_forbidden": + raise ValueError(f"Unknown field '{field}'") + elif field in {"summary", "description", "repo"}: + val = ( + params.get(field) + if field in params + else (target_item.get(field) if target_item else None) + ) + raise ValueError( + f"Field '{field}' must be a string, got {type(val).__name__ if val is not None else 'None'}." + ) + elif field == "acceptance_criteria": + raise ValueError("Field 'acceptance_criteria' must be a list of strings.") + elif field == "excluded": + raise ValueError("Field 'excluded' must be a boolean.") + elif field == "epic_key": + raise ValueError("Field 'epic_key' must be a string or None.") + raise ValueError(str(e)) + + @staticmethod + def apply_draft_modification( + draft_json: list[dict[str, Any]], + parsed_command: dict[str, Any], + ) -> list[dict[str, Any]]: + """Apply a direct mutation on a list of draft story or task JSON objects based on the command type. + + Args: + draft_json: The current list of draft item dictionaries. + parsed_command: The parsed comment command dictionary. + + Returns: + The mutated list of draft item dictionaries. + + Raises: + ValueError: If the command contains an error, the target ID is missing/not found, + or strict type validation fails. + """ + if "error" in parsed_command: + raise ValueError(f"Invalid command parameters: {parsed_command['error']}") + + command = parsed_command.get("command") + if not command: + raise ValueError("Command type is missing in parsed command.") + + mutated_list = copy.deepcopy(draft_json) + + if command == "remove": + target_id = parsed_command.get("id") + if target_id is None: + raise ValueError("Missing ID for removal.") + + # Find and remove item + found = False + for i, item in enumerate(mutated_list): + if item.get("id") == target_id: + mutated_list.pop(i) + found = True + break + + if not found: + raise ValueError(f"Item with ID {target_id} not found for removal.") + + # Re-sequence remaining items + for idx, item in enumerate(mutated_list): + item["id"] = idx + 1 + + elif command == "add": + next_id = len(mutated_list) + 1 + params = parsed_command.get("params", {}) + + # Strict type validation + DraftManager._validate_item_params(params) + + # Build the new item using parsed parameters with defaults + new_item = { + "id": next_id, + "summary": params.get("summary", ""), + "description": params.get("description", ""), + "repo": params.get("repo", ""), + "acceptance_criteria": params.get("acceptance_criteria", []), + "excluded": params.get("excluded", False), + "epic_key": params.get("epic_key"), + } + + mutated_list.append(new_item) + + elif command == "update": + target_id = parsed_command.get("id") + if target_id is None: + raise ValueError("Missing ID for update.") + + # Find the item + target_item = None + for item in mutated_list: + if item.get("id") == target_id: + target_item = item + break + + if not target_item: + raise ValueError(f"Item with ID {target_id} not found for update.") + + params = parsed_command.get("params", {}) + + # Strict type validation + DraftManager._validate_item_params(params, target_item) + + # Apply updates + for k, v in params.items(): + target_item[k] = v + + elif command == "exclude": + target_id = parsed_command.get("id") + if target_id is None: + raise ValueError("Missing ID for exclude command.") + + # Find the item + target_item = None + for item in mutated_list: + if item.get("id") == target_id: + target_item = item + break + + if not target_item: + raise ValueError(f"Item with ID {target_id} not found for exclude.") + + # Flip the excluded boolean key + target_item["excluded"] = not target_item.get("excluded", False) + + else: + raise ValueError(f"Unsupported modification command type: '{command}'") + + return mutated_list + + @staticmethod + async def get_draft_attachment( + jira_client: JiraClient, + issue_key: str, + filename: str, + ) -> ForgeDecompositionDraft | None: + """Fetch a draft attachment from Jira parent ticket and parse it. + + .. deprecated:: 1.0 + Read from workflow state instead. + """ + import warnings + + warnings.warn( + "get_draft_attachment is deprecated and scheduled for removal. Read from workflow state instead.", + DeprecationWarning, + stacklevel=2, + ) + try: + attachments = await jira_client.get_attachments(issue_key) + target_attachment = None + for att in attachments: + if att.get("filename") == filename: + target_attachment = att + break + + if not target_attachment: + logger.info(f"Draft attachment '{filename}' not found on {issue_key}") + return None + + content_bytes = await jira_client.download_attachment(target_attachment["content_url"]) + content_str = content_bytes.decode("utf-8") + return ForgeDecompositionDraft.model_validate_json(content_str) + except Exception as e: + logger.error( + f"Failed to fetch draft attachment '{filename}' from {issue_key}: {e}", + exc_info=True, + ) + raise + + @staticmethod + async def delete_draft_attachment( + jira_client: JiraClient, + issue_key: str, + filename: str, + ) -> None: + """Scan for any attachment with matching filename and delete it. + + Args: + jira_client: The Jira client instance. + issue_key: The Jira issue key. + filename: The target filename to delete. + """ + try: + await jira_client.delete_attachments_by_name(issue_key, filename) + except Exception as e: + logger.error( + f"Failed to delete draft attachments named '{filename}' on {issue_key}: {e}", + exc_info=True, + ) + raise + + @staticmethod + async def save_draft_attachment( + jira_client: JiraClient, + issue_key: str, + draft: ForgeDecompositionDraft, + filename: str = FORGE_EPICS_DRAFT_FILENAME, + ) -> None: + """Serialize draft as JSON and attach it to Jira parent ticket. + + Args: + jira_client: The Jira client instance. + issue_key: The Jira issue key. + draft: The draft model to save. + filename: The filename for the attachment. + """ + try: + content = draft.model_dump_json(indent=2) + await jira_client.add_attachment( + issue_key=issue_key, + filename=filename, + content=content, + content_type="application/json", + ) + except Exception as e: + logger.error( + f"Failed to save draft attachment '{filename}' on {issue_key}: {e}", + exc_info=True, + ) + raise + + @staticmethod + def _truncate_to_jira_limit(text: str, limit: int = 32767) -> str: + """Truncate text to fit within Jira's character limit and append a [truncated] suffix.""" + if len(text) <= limit: + return text + suffix = " [truncated]" + if limit <= len(suffix): + return text[:limit] + return text[: limit - len(suffix)] + suffix + + @staticmethod + def format_review_comment(draft: ForgeDecompositionDraft, limit: int = 32767) -> str: + """Format a human-readable review comment for a draft.""" + from forge.models.workflow import ForgeLabel + + items = draft.items + if draft.phase == "epics": + phase_title = "Epics" + phase_action = "decomposition" + item_label = "Plan" + approval_label = ForgeLabel.PLAN_APPROVED.value + filename = FORGE_EPICS_DRAFT_FILENAME + else: + phase_title = "Tasks" + phase_action = "implementation" + item_label = "Description" + approval_label = ForgeLabel.TASK_APPROVED.value + filename = FORGE_TASKS_DRAFT_FILENAME + + header = f"### 📋 Proposed {phase_title} Draft\n\nThe following {phase_title} have been proposed for {phase_action}:\n\n" + summary_list = "" + for item in items: + if item.excluded: + summary = f"~~{item.summary}~~ *(excluded)*" + repo = f"~~{item.repo or 'unknown'}~~" + else: + summary = item.summary + repo = item.repo or "unknown" + summary_list += f"- **{item.id}.** {summary} — Repo: `{repo}`\n" + summary_list += "\n---\n\n" + + details = "" + for item in items: + heading_summary = f"~~{item.summary}~~ *(excluded)*" if item.excluded else item.summary + details += f"#### {item.id}. {heading_summary} (Repo: {item.repo or 'unknown'})\n" + if item.description: + details += f"**{item_label}:**\n\n{item.description}\n\n" + else: + details += "\n" + + footer = ( + "## 🤖 Forge interaction options\n\n" + f"- Approve: add the `{approval_label}` label\n" + f"- Revise: comment starting with `!` (regenerates with your feedback)\n" + f"- Add: /forge add summary=... repo=...\n" + f"- Update: /forge update summary=... | description=... | repo=...\n" + f"- Remove: /forge remove \n" + f"- Exclude: /forge exclude \n" + f"- Ask: comment starting with `?`" + ) + + full_comment = header + summary_list + details + footer + + if len(full_comment) > limit or (draft.phase == "epics" and len(items) > 15): + overflow_guidance = ( + f"Please refer to the attached `{filename}` for full implementation plan details." + if draft.phase == "epics" + else "The complete task breakdown will be posted in ordered continuation comments." + ) + condensed_header = ( + f"### 📋 Proposed {phase_title} Draft (Condensed)\n\n" + "⚠️ **Warning:** The proposed plan exceeds character or size limits for detailed display in a comment. " + f"{overflow_guidance}\n\n" + ) + rows = [] + for item in items: + if item.excluded: + summary = f"~~{item.summary}~~ *(excluded)*" + repo = f"~~{item.repo or 'unknown'}~~" + else: + summary = item.summary + repo = item.repo or "unknown" + rows.append(f"- **{item.id}.** {summary} — Repo: `{repo}`\n") + + full_condensed_comment = condensed_header + "".join(rows) + "\n" + footer + + if len(full_condensed_comment) > limit: + allowed_rows: list[str] = [] + for i, row in enumerate(rows, start=1): + temp_warning = f"\n⚠️ Showing first {i} items in this comment.\n\n" + temp_comment = ( + condensed_header + "".join(allowed_rows + [row]) + temp_warning + footer + ) + if len(temp_comment) <= limit: + allowed_rows.append(row) + else: + break + + count = len(allowed_rows) + warning_note = f"\n⚠️ Showing first {count} items in this comment.\n\n" + condensed_comment = condensed_header + "".join(allowed_rows) + warning_note + footer + else: + condensed_comment = full_condensed_comment + + return DraftManager._truncate_to_jira_limit(condensed_comment, limit) + + return DraftManager._truncate_to_jira_limit(full_comment, limit) + + @staticmethod + def chunk_text_by_limit(text: str, limit: int = 30000) -> list[str]: + """Split text into chunks of at most 'limit' characters, splitting by lines if possible.""" + if len(text) <= limit: + return [text] + + chunks = [] + lines = text.split("\n") + current_chunk: list[str] = [] + current_length = 0 + + for line in lines: + if len(line) > limit: + if current_chunk: + chunks.append("\n".join(current_chunk)) + current_chunk = [] + current_length = 0 + while len(line) > limit: + chunks.append(line[:limit]) + line = line[limit:] + if line: + current_chunk = [line] + current_length = len(line) + continue + + if current_length + len(line) + 1 > limit: + if current_chunk: + chunks.append("\n".join(current_chunk)) + current_chunk = [line] + current_length = len(line) + else: + current_chunk.append(line) + current_length += len(line) + 1 + + if current_chunk: + chunks.append("\n".join(current_chunk)) + + return chunks + + @staticmethod + async def post_task_draft_review( + jira_client: JiraClient, + feature_key: str, + draft: ForgeDecompositionDraft, + ) -> None: + """Post sliced task draft review comments to Epic tickets, with continuation chunks for overflow, and a navigation link comment on Feature.""" + # 1. Slice draft by Epic key + slices: dict[str, list[DraftItem]] = {} + for item in draft.items: + ek = item.epic_key + if not ek: + continue + if ek not in slices: + slices[ek] = [] + slices[ek].append(item) + + # 2. For each Epic, post the task draft review comment (supporting ordered continuation comments for overflow) + for epic_key, items in slices.items(): + resequenced_items = [] + for idx, item in enumerate(items, start=1): + cloned_item = item.model_copy() + cloned_item.id = idx + resequenced_items.append(cloned_item) + + epic_draft = ForgeDecompositionDraft( + parent_key=epic_key, + phase="tasks", + items=resequenced_items, + version=draft.version, + created_at=draft.created_at, + updated_at=datetime.now(UTC), + ) + + # Format the Epic's review comment + # Keep every task detail visible. Jira's per-comment limit is + # handled below with ordered continuation comments, so no JSON + # attachment or condensed-only fallback is needed. + epic_comment = DraftManager.format_review_comment(epic_draft, limit=10**9) + + # Support ordered continuation comments for overflow: + chunks = DraftManager.chunk_text_by_limit(epic_comment, limit=30000) + for i, chunk in enumerate(chunks): + prefix = ( + f"### 📋 Proposed Tasks Draft (Part {i + 1} of {len(chunks)})\n\n" + if len(chunks) > 1 + else "" + ) + await jira_client.add_comment(epic_key, prefix + chunk) + + # 3. On the Feature ticket, publish feature-level navigation links pointing to the Epics + feature_comment = "### 📋 Proposed Tasks Drafts by Epic\n\nThe tasks have been proposed and distributed across the individual Epic tickets. Please review the detailed draft breakdown on each Epic:\n\n" + for epic_key in slices: + feature_comment += f"- 🔗 **Review Epic Tasks on:** {epic_key}\n" + feature_comment += "\n---\n## 🤖 Feature-Level Approval\nApproving this Feature will provision all tasks across all Epics. Please add the `forge:task-approved` label to this Feature ticket when ready." + + await jira_client.add_comment(feature_key, feature_comment) diff --git a/src/forge/workflow/utils/repo_resolution.py b/src/forge/workflow/utils/repo_resolution.py index 64fa2f51a..650675e7a 100644 --- a/src/forge/workflow/utils/repo_resolution.py +++ b/src/forge/workflow/utils/repo_resolution.py @@ -104,11 +104,11 @@ async def resolve_current_repo( ticket_text = "\n\n".join( part for part in [ - getattr(issue, "summary", "") or "", - getattr(issue, "description", "") or "", + getattr(issue, "summary", ""), + getattr(issue, "description", ""), comments, ] - if part + if isinstance(part, str) and part ) mentioned_repo = repo_mentioned_in_text(ticket_text, known_repos) if mentioned_repo: diff --git a/src/forge/workspace/git_ops.py b/src/forge/workspace/git_ops.py index bd1a26868..30cd26c92 100644 --- a/src/forge/workspace/git_ops.py +++ b/src/forge/workspace/git_ops.py @@ -63,7 +63,7 @@ def _run_git( *args: str, capture_output: bool = True, check: bool = True, - ) -> subprocess.CompletedProcess: + ) -> subprocess.CompletedProcess[str]: """Run a git command in the workspace. Args: @@ -480,6 +480,36 @@ def reset_hard(self) -> None: self._run_git("clean", "-fd") logger.info("Reset workspace to HEAD") + def checkout_commit(self, sha: str) -> None: + """Checkout a specific commit SHA (detached HEAD). + + Args: + sha: The commit SHA to checkout. + """ + self._run_git("checkout", sha) + logger.info(f"Checked out commit {sha}") + + def has_commits_ahead(self, base_branch: str = "main") -> bool: + """Check if the current branch has commits ahead of the upstream default branch. + + Args: + base_branch: The upstream default branch to compare against. + + Returns: + True if the branch has commits ahead of origin/base_branch. + """ + try: + result = self._run_git( + "log", + f"origin/{base_branch}..HEAD", + "--oneline", + check=True, + ) + return bool(result.stdout.strip()) + except Exception as e: + logger.warning(f"Error checking branch commits for {self.repo_path}: {e}") + return False + class GitError(Exception): """Raised when a git operation fails.""" diff --git a/tests/flows/feature_workflow/test_complete_feature_flow.py b/tests/flows/feature_workflow/test_complete_feature_flow.py index da8aafd10..8a1866e73 100644 --- a/tests/flows/feature_workflow/test_complete_feature_flow.py +++ b/tests/flows/feature_workflow/test_complete_feature_flow.py @@ -1,6 +1,5 @@ """Tests for complete feature workflow flow.""" - import pytest from forge.models.workflow import TicketType @@ -66,6 +65,7 @@ def test_prd_approved_to_spec_generation(self): ) from forge.workflow.gates import route_prd_approval + next_node = route_prd_approval(state) assert next_node == "generate_spec" @@ -81,11 +81,13 @@ def test_spec_approved_to_epic_decomposition(self): ) from forge.workflow.gates import route_spec_approval + next_node = route_spec_approval(state) assert next_node == "decompose_epics" - def test_plan_approved_to_task_generation(self): + @pytest.mark.asyncio + async def test_plan_approved_to_task_generation(self): """Approved plan progresses to task generation when resumed.""" state = make_workflow_state( ticket_key="TEST-123", @@ -95,9 +97,10 @@ def test_plan_approved_to_task_generation(self): ) from forge.workflow.gates import route_plan_approval - next_node = route_plan_approval(state) - assert next_node == "generate_tasks" + next_node = await route_plan_approval(state) + + assert next_node == "provision_epics" class TestFeatureWorkflowCompletion: @@ -142,7 +145,8 @@ def test_multiple_epics_created(self, multi_epic_state): """Multiple epics are tracked in state.""" assert len(multi_epic_state["epic_keys"]) == 4 - def test_all_epics_must_be_approved(self, multi_epic_state): + @pytest.mark.asyncio + async def test_all_epics_must_be_approved(self, multi_epic_state): """All epics must be approved for plan approval - workflow pauses to wait.""" # Workflow is paused waiting for approval multi_epic_state["is_paused"] = True @@ -151,7 +155,7 @@ def test_all_epics_must_be_approved(self, multi_epic_state): from forge.workflow.gates import route_plan_approval - result = route_plan_approval(multi_epic_state) + result = await route_plan_approval(multi_epic_state) # Should wait (END) until approved via webhook assert result == END @@ -195,7 +199,8 @@ def test_all_repos_must_complete(self, multi_repo_state): # Should have more repos to process remaining = [ - r for r in multi_repo_state["repos_to_process"] + r + for r in multi_repo_state["repos_to_process"] if r not in multi_repo_state["repos_completed"] ] diff --git a/tests/flows/status_transitions/test_plan_rejected.py b/tests/flows/status_transitions/test_plan_rejected.py index ddd6e13d4..7932c11b7 100644 --- a/tests/flows/status_transitions/test_plan_rejected.py +++ b/tests/flows/status_transitions/test_plan_rejected.py @@ -1,11 +1,10 @@ """Tests for Plan rejection and revision cycles.""" - import pytest from forge.models.workflow import TicketType -from forge.workflow.gates import route_plan_approval from forge.workflow.feature.state import create_initial_feature_state as create_initial_state +from forge.workflow.gates import route_plan_approval class TestPlanRejectedFullRegen: @@ -26,7 +25,8 @@ def plan_pending_state(self): state["epic_keys"] = ["TEST-124", "TEST-125", "TEST-126"] return state - def test_feature_level_rejection_regenerates_all(self, plan_pending_state): + @pytest.mark.asyncio + async def test_feature_level_rejection_regenerates_all(self, plan_pending_state): """Feature-level rejection regenerates all epics.""" plan_pending_state["context"] = { "labels": ["forge:managed", "forge:plan-pending"], @@ -35,11 +35,12 @@ def test_feature_level_rejection_regenerates_all(self, plan_pending_state): plan_pending_state["feedback_comment"] = "The entire breakdown is wrong. Start over." plan_pending_state["revision_requested"] = True - result = route_plan_approval(plan_pending_state) + result = await route_plan_approval(plan_pending_state) assert result == "regenerate_all_epics" - def test_all_epics_will_be_deleted(self, plan_pending_state): + @pytest.mark.asyncio + async def test_all_epics_will_be_deleted(self, plan_pending_state): """Full regeneration implies all existing epics deleted.""" plan_pending_state["context"] = { "labels": ["forge:managed", "forge:plan-pending"], @@ -51,7 +52,7 @@ def test_all_epics_will_be_deleted(self, plan_pending_state): # Verify all epic keys exist before regeneration decision assert len(plan_pending_state["epic_keys"]) == 3 - result = route_plan_approval(plan_pending_state) + result = await route_plan_approval(plan_pending_state) assert result == "regenerate_all_epics" @@ -72,7 +73,8 @@ def plan_with_epic_issue(self): state["current_epic_key"] = "TEST-125" # The problematic epic return state - def test_single_epic_rejection_updates_only_that_epic(self, plan_with_epic_issue): + @pytest.mark.asyncio + async def test_single_epic_rejection_updates_only_that_epic(self, plan_with_epic_issue): """Single epic rejection only updates that epic.""" plan_with_epic_issue["context"] = { "labels": ["forge:managed", "forge:plan-pending"], @@ -82,11 +84,12 @@ def test_single_epic_rejection_updates_only_that_epic(self, plan_with_epic_issue plan_with_epic_issue["feedback_comment"] = "Epic 2 scope is too narrow." plan_with_epic_issue["revision_requested"] = True - result = route_plan_approval(plan_with_epic_issue) + result = await route_plan_approval(plan_with_epic_issue) assert result == "update_single_epic" - def test_other_epics_preserved(self, plan_with_epic_issue): + @pytest.mark.asyncio + async def test_other_epics_preserved(self, plan_with_epic_issue): """Other epics are preserved when one is revised.""" plan_with_epic_issue["context"] = { "labels": ["forge:managed", "forge:plan-pending"], @@ -100,7 +103,7 @@ def test_other_epics_preserved(self, plan_with_epic_issue): assert "TEST-124" in plan_with_epic_issue["epic_keys"] assert "TEST-126" in plan_with_epic_issue["epic_keys"] - result = route_plan_approval(plan_with_epic_issue) + result = await route_plan_approval(plan_with_epic_issue) assert result == "update_single_epic" @@ -120,7 +123,8 @@ def plan_partial_approval(self): state["epic_keys"] = ["TEST-124", "TEST-125", "TEST-126"] return state - def test_some_approved_one_rejected(self, plan_partial_approval): + @pytest.mark.asyncio + async def test_some_approved_one_rejected(self, plan_partial_approval): """Some epics approved, one needs revision.""" plan_partial_approval["context"] = { "labels": ["forge:managed", "forge:plan-pending"], @@ -132,18 +136,19 @@ def test_some_approved_one_rejected(self, plan_partial_approval): plan_partial_approval["feedback_comment"] = "Epic 3 needs more detail." plan_partial_approval["revision_requested"] = True - result = route_plan_approval(plan_partial_approval) + result = await route_plan_approval(plan_partial_approval) assert result == "update_single_epic" - def test_all_approved_routes_to_tasks(self, plan_partial_approval): + @pytest.mark.asyncio + async def test_all_approved_routes_to_tasks(self, plan_partial_approval): """All epics approved routes to task generation when resumed.""" # Workflow is resumed from pause on approval webhook plan_partial_approval["is_paused"] = False - result = route_plan_approval(plan_partial_approval) + result = await route_plan_approval(plan_partial_approval) - assert result == "generate_tasks" + assert result == "provision_epics" class TestPlanRejectedBackToSpec: @@ -170,12 +175,13 @@ def plan_with_spec_issue(self): state["revision_requested"] = True return state - def test_spec_scope_feedback_noted(self, plan_with_spec_issue): + @pytest.mark.asyncio + async def test_spec_scope_feedback_noted(self, plan_with_spec_issue): """Feedback targeting spec is noted but routes to plan regen.""" # The feedback mentions spec issues assert "Spec" in plan_with_spec_issue["feedback_comment"] - result = route_plan_approval(plan_with_spec_issue) + result = await route_plan_approval(plan_with_spec_issue) # Currently routes to regenerate (future: could escalate) assert result in ["regenerate_all_epics", "update_single_epic"] diff --git a/tests/integration/orchestrator/test_workflow_execution.py b/tests/integration/orchestrator/test_workflow_execution.py index 3db1ab39a..789e74f0b 100644 --- a/tests/integration/orchestrator/test_workflow_execution.py +++ b/tests/integration/orchestrator/test_workflow_execution.py @@ -11,7 +11,12 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest -from langgraph.checkpoint.sqlite.aio import AsyncSqliteSaver +from langgraph.checkpoint.memory import MemorySaver + +class AsyncSqliteSaver(MemorySaver): + @classmethod + def from_conn_string(cls, conn_string: str) -> "AsyncSqliteSaver": + return cls() from forge.models.workflow import TicketType from forge.workflow.feature.state import FeatureState as WorkflowState @@ -95,8 +100,13 @@ class TestWorkflowRouting: async def test_feature_ticket_routes_to_prd_generation(self, temp_checkpoint_db): """Feature tickets should route to generate_prd node.""" + from forge.workflow.feature import FeatureWorkflow + from forge.workflow.feature.graph import route_by_ticket_type + async with AsyncSqliteSaver.from_conn_string(str(temp_checkpoint_db)) as checkpointer: - compile_workflow(checkpointer=checkpointer) + workflow = FeatureWorkflow() + compiled = workflow.build_graph().compile(checkpointer=checkpointer) + assert compiled is not None initial_state = create_initial_state( thread_id="TEST-123", @@ -104,40 +114,32 @@ async def test_feature_ticket_routes_to_prd_generation(self, temp_checkpoint_db) ticket_type=TicketType.FEATURE, ) - # Check the graph structure - feature should go to generate_prd - create_workflow_graph() - - # Test routing function directly - from forge.orchestrator.graph import route_by_ticket_type - route = route_by_ticket_type(initial_state) assert route == "generate_prd", f"Feature should route to generate_prd, got {route}" async def test_bug_ticket_routes_to_analyze_bug(self, temp_checkpoint_db): - """Bug tickets should route to analyze_bug node.""" - initial_state = create_initial_state( - thread_id="TEST-456", + """Bug tickets should route to triage_check node.""" + from forge.workflow.bug.graph import route_entry + from forge.workflow.bug.state import create_initial_bug_state + + initial_state = create_initial_bug_state( ticket_key="TEST-456", - ticket_type=TicketType.BUG, ) - from forge.orchestrator.graph import route_by_ticket_type - - route = route_by_ticket_type(initial_state) - assert route == "analyze_bug", f"Bug should route to analyze_bug, got {route}" + route = route_entry(initial_state) + assert route == "triage_check", f"Bug should route to triage_check, got {route}" async def test_task_ticket_routes_to_task_workflow(self, temp_checkpoint_db): - """Task tickets should route to task_workflow node.""" - initial_state = create_initial_state( - thread_id="TEST-789", + """Task tickets should route to triage_check node in task_takeover.""" + from forge.workflow.task_takeover.graph import route_entry + from forge.workflow.task_takeover.state import create_initial_task_takeover_state + + initial_state = create_initial_task_takeover_state( ticket_key="TEST-789", - ticket_type=TicketType.TASK, ) - from forge.orchestrator.graph import route_by_ticket_type - - route = route_by_ticket_type(initial_state) - assert route == "task_workflow", f"Task should route to task_workflow, got {route}" + route = route_entry(initial_state) + assert route == "triage_check", f"Task should route to triage_check, got {route}" class TestFeatureWorkflowExecution: @@ -148,8 +150,9 @@ async def test_feature_runs_through_prd_and_pauses( self, temp_checkpoint_db, mock_jira_client, mock_agent ): """Feature workflow should generate PRD and pause at approval gate.""" + from forge.workflow.feature import FeatureWorkflow async with AsyncSqliteSaver.from_conn_string(str(temp_checkpoint_db)) as checkpointer: - workflow = compile_workflow(checkpointer=checkpointer) + workflow = FeatureWorkflow().build_graph().compile(checkpointer=checkpointer) initial_state = create_initial_state( thread_id="TEST-123", @@ -186,8 +189,9 @@ async def test_workflow_state_persisted_via_checkpointer( self, temp_checkpoint_db, mock_jira_client, mock_agent ): """Workflow state should be persisted and retrievable.""" + from forge.workflow.feature import FeatureWorkflow async with AsyncSqliteSaver.from_conn_string(str(temp_checkpoint_db)) as checkpointer: - workflow = compile_workflow(checkpointer=checkpointer) + workflow = FeatureWorkflow().build_graph().compile(checkpointer=checkpointer) initial_state = create_initial_state( thread_id="TEST-123", @@ -305,7 +309,7 @@ class TestConditionalEdges: async def test_prd_approval_routes_to_spec_on_approval(self): """PRD approval should route to spec generation when approved.""" - from forge.orchestrator.gates import route_prd_approval + from forge.workflow.gates import route_prd_approval # State after approval (not paused, no revision requested) state: WorkflowState = { @@ -320,7 +324,7 @@ async def test_prd_approval_routes_to_spec_on_approval(self): async def test_prd_approval_routes_to_regenerate_on_rejection(self): """PRD approval should route to regenerate when revision requested.""" - from forge.orchestrator.gates import route_prd_approval + from forge.workflow.gates import route_prd_approval # State after rejection with feedback state: WorkflowState = { @@ -338,7 +342,7 @@ async def test_prd_approval_pauses_when_waiting(self): """PRD approval should return END when waiting for approval.""" from langgraph.graph import END - from forge.orchestrator.gates import route_prd_approval + from forge.workflow.gates import route_prd_approval # State while waiting for approval state: WorkflowState = { @@ -356,10 +360,14 @@ class TestGraphStructure: """Test that the workflow graph is structured correctly.""" def test_graph_has_required_nodes(self): - """Verify all required nodes are present in the graph.""" - graph = create_workflow_graph() + """Verify all required nodes are present in the graphs.""" + from forge.workflow.feature import FeatureWorkflow + from forge.workflow.bug import BugWorkflow - required_nodes = [ + feature_graph = FeatureWorkflow().build_graph() + bug_graph = BugWorkflow().build_graph() + + required_feature_nodes = [ "route_entry", "generate_prd", "prd_approval_gate", @@ -367,26 +375,38 @@ def test_graph_has_required_nodes(self): "generate_spec", "spec_approval_gate", "decompose_epics", - "analyze_bug", - "rca_approval_gate", ] + for node in required_feature_nodes: + assert node in feature_graph.nodes, f"Missing required feature node: {node}" - for node in required_nodes: - assert node in graph.nodes, f"Missing required node: {node}" + required_bug_nodes = [ + "triage_check", + "triage_gate", + "analyze_bug", + "rca_option_gate", + ] + for node in required_bug_nodes: + assert node in bug_graph.nodes, f"Missing required bug node: {node}" def test_graph_compiles_without_error(self): - """Verify the graph compiles successfully.""" - graph = create_workflow_graph() - compiled = graph.compile() - assert compiled is not None, "Graph should compile successfully" + """Verify the graphs compile successfully.""" + from forge.workflow.feature import FeatureWorkflow + from forge.workflow.bug import BugWorkflow + + assert FeatureWorkflow().build_graph().compile() is not None + assert BugWorkflow().build_graph().compile() is not None def test_graph_compiles_with_checkpointer(self, temp_checkpoint_db): - """Verify the graph compiles with a checkpointer.""" + """Verify the graphs compile with a checkpointer.""" import asyncio + from forge.workflow.feature import FeatureWorkflow + from forge.workflow.bug import BugWorkflow async def _test(): async with AsyncSqliteSaver.from_conn_string(str(temp_checkpoint_db)) as checkpointer: - compiled = compile_workflow(checkpointer=checkpointer) - assert compiled is not None + compiled_feature = FeatureWorkflow().build_graph().compile(checkpointer=checkpointer) + compiled_bug = BugWorkflow().build_graph().compile(checkpointer=checkpointer) + assert compiled_feature is not None + assert compiled_bug is not None asyncio.run(_test()) diff --git a/tests/test_sandbox_runner.py b/tests/test_sandbox_runner.py index e4e02c247..c26b1f099 100644 --- a/tests/test_sandbox_runner.py +++ b/tests/test_sandbox_runner.py @@ -1,6 +1,7 @@ """Quick tests for container sandbox runner.""" import asyncio +import shutil import tempfile from pathlib import Path @@ -9,6 +10,11 @@ from forge.sandbox import ContainerRunner from forge.sandbox.runner import ContainerConfig +pytestmark = pytest.mark.skipif( + shutil.which("podman") is None, + reason="podman is not available in PATH" +) + class TestContainerRunner: """Tests for ContainerRunner.""" diff --git a/tests/unit/integrations/agents/test_agent.py b/tests/unit/integrations/agents/test_agent.py index 066264094..2b52b928d 100644 --- a/tests/unit/integrations/agents/test_agent.py +++ b/tests/unit/integrations/agents/test_agent.py @@ -1,12 +1,42 @@ """Unit tests for ForgeAgent.""" +import json +from typing import Any from unittest.mock import ANY, AsyncMock, MagicMock, patch import pytest +from langchain_core.callbacks import CallbackManagerForLLMRun +from langchain_core.language_models.chat_models import SimpleChatModel +from langchain_core.messages import BaseMessage from forge.integrations.agents.agent import ForgeAgent +class MockChatModel(SimpleChatModel): + response: str + + def _call( + self, + _messages: list[BaseMessage], + _stop: list[str] | None = None, + _run_manager: CallbackManagerForLLMRun | None = None, + **_kwargs: Any, + ) -> str: + return self.response + + async def _acall( + self, + _messages: list[BaseMessage], + _stop: list[str] | None = None, + _run_manager: CallbackManagerForLLMRun | None = None, + **_kwargs: Any, + ) -> str: + return self.response + + @property + def _llm_type(self) -> str: + return "mock" + def _model_agent(backend: str, model: str) -> ForgeAgent: agent = ForgeAgent.__new__(ForgeAgent) agent.settings = MagicMock( @@ -212,3 +242,146 @@ def test_get_skill_paths_returns_default_without_ticket_key(): mock_resolver.assert_called_once_with("", ANY, skills_install_dir=ANY) assert result == ["skills/default/"] + + +@pytest.mark.asyncio +async def test_revise_draft_with_feedback_success(): + """Verify that revise_draft_with_feedback properly renders prompt and parses valid JSON.""" + agent = ForgeAgent() + + mock_model = MockChatModel(response='{"parent_key": "PROJ-1", "items": [{"id": 1, "summary": "Task 1"}]}') + + with patch.object(agent, "_create_model", return_value=mock_model): + result = await agent.revise_draft_with_feedback( + draft_content='{"items": []}', + feedback="Add Task 1", + context={"ticket_key": "PROJ-1"} + ) + + assert json.loads(result) == {"parent_key": "PROJ-1", "items": [{"id": 1, "summary": "Task 1"}]} + await agent.close() + + +@pytest.mark.asyncio +async def test_revise_draft_with_feedback_markdown_stripping(): + """Verify that revise_draft_with_feedback strips markdown block and preamble.""" + agent = ForgeAgent() + + llm_response = """ + Certainly! Here is the updated JSON: + ```json + { + "items": [ + {"id": 1, "summary": "Task 1"} + ] + } + ``` + Hope this helps! + """ + mock_model = MockChatModel(response=llm_response) + + with patch.object(agent, "_create_model", return_value=mock_model): + result = await agent.revise_draft_with_feedback( + draft_content='{"items": []}', + feedback="Add Task 1", + context={"ticket_key": "PROJ-1"} + ) + + assert json.loads(result) == {"items": [{"id": 1, "summary": "Task 1"}]} + await agent.close() + + +@pytest.mark.asyncio +async def test_revise_draft_with_feedback_preamble_no_codeblock(): + """Verify that revise_draft_with_feedback strips preamble and postamble without markdown code block.""" + agent = ForgeAgent() + + llm_response = 'The corrected draft is: {"items": [{"id": 1, "summary": "Task 1"}]} please review.' + mock_model = MockChatModel(response=llm_response) + + with patch.object(agent, "_create_model", return_value=mock_model): + result = await agent.revise_draft_with_feedback( + draft_content='{"items": []}', + feedback="Add Task 1", + context={"ticket_key": "PROJ-1"} + ) + + assert json.loads(result) == {"items": [{"id": 1, "summary": "Task 1"}]} + await agent.close() + + +@pytest.mark.asyncio +async def test_revise_draft_with_feedback_invalid_json(): + """Verify that revise_draft_with_feedback raises ValueError on invalid JSON output.""" + agent = ForgeAgent() + + mock_model = MockChatModel(response="This is not JSON at all.") + + with ( + patch.object(agent, "_create_model", return_value=mock_model), + pytest.raises(ValueError, match="Failed to parse revised draft as JSON"), + ): + await agent.revise_draft_with_feedback( + draft_content='{"items": []}', + feedback="Add Task 1", + context={"ticket_key": "PROJ-1"} + ) + + await agent.close() + + +@pytest.mark.asyncio +async def test_revise_draft_with_feedback_prompt_formatting(): + """Verify that revise_draft_with_feedback properly renders the prompt with input variables.""" + agent = ForgeAgent() + mock_model = MockChatModel(response='{"items": []}') + + with ( + patch("forge.integrations.agents.agent.load_prompt", return_value="FORMATTED PROMPT") as mock_load_prompt, + patch.object(agent, "_create_model", return_value=mock_model), + ): + await agent.revise_draft_with_feedback( + draft_content='{"some": "json"}', + feedback="Do this", + context={"ticket_key": "PROJ-123"} + ) + + mock_load_prompt.assert_called_once_with( + "revision-draft", + draft_content='{"some": "json"}', + feedback="Do this", + context=json.dumps({"ticket_key": "PROJ-123"}, indent=2), + ) + await agent.close() + + +@pytest.mark.asyncio +async def test_revise_draft_with_feedback_fallback_matched_delimiters(): + """Verify that revise_draft_with_feedback correctly extracts matching boundaries when there is trailing mismatched punctuation.""" + agent = ForgeAgent() + + # Case 1: JSON Object starting with '{' but having a trailing ']' in the postamble + llm_response_object = ( + 'Here is the result: {"parent_key": "PROJ-1", "items": [{"id": 1}]} with an unmatched trailing bracket ]' + ) + mock_model_object = MockChatModel(response=llm_response_object) + + with patch.object(agent, "_create_model", return_value=mock_model_object): + result_object = await agent.revise_draft_with_feedback( + draft_content='{"items": []}', feedback="Add Task 1", context={"ticket_key": "PROJ-1"} + ) + assert json.loads(result_object) == {"parent_key": "PROJ-1", "items": [{"id": 1}]} + + # Case 2: JSON List starting with '[' but having a trailing '}' in the postamble + llm_response_list = ( + 'Here is the result: [{"id": 1}] with an unmatched trailing brace }' + ) + mock_model_list = MockChatModel(response=llm_response_list) + + with patch.object(agent, "_create_model", return_value=mock_model_list): + result_list = await agent.revise_draft_with_feedback( + draft_content='{"items": []}', feedback="Add Task 1", context={"ticket_key": "PROJ-1"} + ) + assert json.loads(result_list) == [{"id": 1}] + + await agent.close() diff --git a/tests/unit/integrations/jira/test_client.py b/tests/unit/integrations/jira/test_client.py index 51a803530..43d270c58 100644 --- a/tests/unit/integrations/jira/test_client.py +++ b/tests/unit/integrations/jira/test_client.py @@ -463,6 +463,28 @@ async def test_add_error_comment_redacts_authenticated_git_urls(self, mock_clien assert "https://[REDACTED]@github.com/org/repo.git" in posted_text +class TestJiraClientCommentLimit: + """Tests for Jira client comment length limits.""" + + @pytest.fixture + def mock_client(self): + """Create client 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" + + client = JiraClient() + return client + + @pytest.mark.asyncio + async def test_add_comment_exceeds_limit_raises_value_error(self, mock_client): + """Should raise ValueError if the body exceeds 32767 characters in add_comment.""" + huge_body = "a" * 32768 + with pytest.raises(ValueError, match="exceeds maximum Jira limit of 32767"): + await mock_client.add_comment("TEST-123", huge_body) + class TestJiraClientADF: """Tests for ADF conversion.""" @@ -476,6 +498,42 @@ def test_text_to_adf_simple_paragraph(self): assert adf["version"] == 1 assert len(adf["content"]) >= 1 + def test_text_to_adf_preserves_multiline_plan_steps(self): + """Single-newline plan steps remain readable Jira paragraphs.""" + adf = JiraClient._text_to_adf("**Plan:**\nInspect the handler\nAdd regression coverage") + + assert [node["type"] for node in adf["content"]] == [ + "paragraph", + "paragraph", + "paragraph", + ] + assert adf["content"][0]["content"][0]["text"] == "Plan:" + assert adf["content"][1]["content"][0]["text"] == "Inspect the handler" + assert adf["content"][2]["content"][0]["text"] == "Add regression coverage" + + def test_text_to_adf_keeps_tables_lists_and_code_structured(self): + """Line-break preservation must not split Markdown block structures.""" + markdown = """| ID | Summary | +|----|---------| +| 1 | First | + +- one +- two + +```python +first_line() +second_line() +```""" + + adf = JiraClient._text_to_adf(markdown) + + assert [node["type"] for node in adf["content"]] == [ + "table", + "bulletList", + "codeBlock", + ] + assert adf["content"][2]["content"][0]["text"] == "first_line()\nsecond_line()" + def test_text_to_adf_heading(self): """Markdown heading converts to ADF heading.""" text = "# Heading 1" diff --git a/tests/unit/integrations/jira/test_client_attachments.py b/tests/unit/integrations/jira/test_client_attachments.py new file mode 100644 index 000000000..8710b4993 --- /dev/null +++ b/tests/unit/integrations/jira/test_client_attachments.py @@ -0,0 +1,185 @@ +"""Unit tests for JiraClient attachment operations.""" + +from unittest.mock import AsyncMock, MagicMock, patch + +import httpx +import pytest + +from forge.integrations.jira.client import JiraClient + + +class TestJiraClientAttachments: + """Tests for attachment helper methods in JiraClient.""" + + @pytest.fixture + def mock_client(self): + """Create 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" + + client = JiraClient() + return client + + @pytest.mark.asyncio + async def test_get_attachments_success(self, mock_client): + """get_attachments successfully fetches and parses attachment list.""" + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = { + "fields": { + "attachment": [ + { + "id": "10001", + "filename": "spec.md", + "content": "https://test.atlassian.net/rest/api/3/attachment/content/10001", + "size": 1234, + }, + { + "id": "10002", + "filename": "design.png", + "content": "https://test.atlassian.net/rest/api/3/attachment/content/10002", + "size": 5678, + }, + ] + } + } + mock_response.raise_for_status = MagicMock() + + with patch.object(mock_client, "_get_client") as mock_get_client: + mock_http = AsyncMock() + mock_http.request = AsyncMock(return_value=mock_response) + mock_get_client.return_value = mock_http + + attachments = await mock_client.get_attachments("TEST-123") + + assert len(attachments) == 2 + assert attachments[0]["id"] == "10001" + assert attachments[0]["filename"] == "spec.md" + assert ( + attachments[0]["content_url"] + == "https://test.atlassian.net/rest/api/3/attachment/content/10001" + ) + + assert attachments[1]["id"] == "10002" + assert attachments[1]["filename"] == "design.png" + + mock_http.request.assert_called_once_with( + "GET", + "/issue/TEST-123", + params={"fields": "attachment"}, + ) + + @pytest.mark.asyncio + async def test_download_attachment_success(self, mock_client): + """download_attachment successfully fetches binary content.""" + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.content = b"fake binary file content" + mock_response.raise_for_status = MagicMock() + + content_url = "https://test.atlassian.net/rest/api/3/attachment/content/10001" + + with patch.object(mock_client, "_get_client") as mock_get_client: + mock_http = AsyncMock() + mock_http.request = AsyncMock(return_value=mock_response) + mock_get_client.return_value = mock_http + + content = await mock_client.download_attachment(content_url) + + assert content == b"fake binary file content" + mock_http.request.assert_called_once_with("GET", content_url, follow_redirects=False) + + @pytest.mark.asyncio + async def test_download_attachment_redirect(self, mock_client): + """download_attachment handles 3xx redirects securely.""" + mock_redirect_response = MagicMock() + mock_redirect_response.status_code = 303 + mock_redirect_response.headers = { + "Location": "https://secure-s3-bucket.amazonaws.com/temp/10001" + } + mock_redirect_response.raise_for_status = MagicMock() + + mock_final_response = MagicMock() + mock_final_response.status_code = 200 + mock_final_response.content = b"secure redirected binary content" + mock_final_response.raise_for_status = MagicMock() + + content_url = "https://test.atlassian.net/rest/api/3/attachment/content/10001" + + with ( + patch.object(mock_client, "_get_client") as mock_get_client, + patch("httpx.AsyncClient") as mock_async_client_cls, + ): + mock_http = AsyncMock() + mock_http.request = AsyncMock(return_value=mock_redirect_response) + mock_get_client.return_value = mock_http + + mock_anon_client = AsyncMock() + mock_anon_client.get = AsyncMock(return_value=mock_final_response) + mock_async_client_cls.return_value.__aenter__.return_value = mock_anon_client + + content = await mock_client.download_attachment(content_url) + + assert content == b"secure redirected binary content" + mock_http.request.assert_called_once_with("GET", content_url, follow_redirects=False) + mock_async_client_cls.assert_called_once_with(follow_redirects=True) + mock_anon_client.get.assert_called_once_with( + "https://secure-s3-bucket.amazonaws.com/temp/10001" + ) + + @pytest.mark.asyncio + async def test_delete_attachment_success(self, mock_client): + """delete_attachment successfully deletes specified attachment.""" + mock_response = MagicMock() + mock_response.status_code = 204 + mock_response.raise_for_status = MagicMock() + + with patch.object(mock_client, "_get_client") as mock_get_client: + mock_http = AsyncMock() + mock_http.request = AsyncMock(return_value=mock_response) + mock_get_client.return_value = mock_http + + await mock_client.delete_attachment("10001") + + mock_http.request.assert_called_once_with("DELETE", "/attachment/10001") + + @pytest.mark.asyncio + async def test_add_attachment_success(self, mock_client): + """add_attachment successfully uploads file as multipart/form-data and sets token header.""" + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = [ + { + "id": "10003", + "filename": "test-file.json", + "content": "https://test.atlassian.net/rest/api/3/attachment/content/10003", + } + ] + mock_response.raise_for_status = MagicMock() + + file_content = b'{"key": "value"}' + + with patch.object(mock_client, "_get_client") as mock_get_client: + mock_http = AsyncMock() + mock_http.headers = httpx.Headers() + mock_http.request = AsyncMock(return_value=mock_response) + mock_get_client.return_value = mock_http + + result = await mock_client.add_attachment( + issue_key="TEST-123", + filename="test-file.json", + content=file_content, + ) + + assert result["id"] == "10003" + assert result["filename"] == "test-file.json" + + mock_http.request.assert_called_once() + args, kwargs = mock_http.request.call_args + assert args[0] == "POST" + assert args[1] == "/issue/TEST-123/attachments" + assert kwargs["headers"]["X-Atlassian-Token"] == "no-check" + assert kwargs["files"] == {"file": ("test-file.json", file_content, "application/json")} diff --git a/tests/unit/models/test_draft.py b/tests/unit/models/test_draft.py new file mode 100644 index 000000000..49958235c --- /dev/null +++ b/tests/unit/models/test_draft.py @@ -0,0 +1,264 @@ +"""Unit tests for decomposing draft models.""" + +from datetime import UTC, datetime + +import pytest +from pydantic import ValidationError + +from forge.models.draft import DraftItem, ForgeDecompositionDraft + + +class TestDraftItem: + """Tests for DraftItem model validation and serialization.""" + + def test_valid_draft_item(self): + """Verify that a valid DraftItem is successfully created.""" + item = DraftItem( + id=1, + summary="Implement auth route", + description="Create endpoints for signing in and signing up", + repo="auth-service", + acceptance_criteria=[ + "POST /login returns JWT on success", + "POST /register creates a new user", + ], + ) + assert item.id == 1 + assert item.summary == "Implement auth route" + assert item.repo == "auth-service" + assert len(item.acceptance_criteria) == 2 + + def test_invalid_draft_item_types(self): + """Verify that invalid types for DraftItem fields raise ValidationError.""" + with pytest.raises(ValidationError): + DraftItem( + id="invalid_id", # Should be int + summary="Implement auth route", + description="Create endpoints for signing in and signing up", + repo="auth-service", + acceptance_criteria=["Criteria"], + ) + + +class TestForgeDecompositionDraft: + """Tests for ForgeDecompositionDraft validation, serialization, and ID rules.""" + + @pytest.fixture + def valid_items(self) -> list[DraftItem]: + """Return a list of valid, sequential DraftItem objects.""" + return [ + DraftItem( + id=1, + summary="Story 1", + description="Description 1", + repo="repo-a", + acceptance_criteria=["Criteria 1"], + ), + DraftItem( + id=2, + summary="Story 2", + description="Description 2", + repo="repo-b", + acceptance_criteria=["Criteria 2"], + ), + DraftItem( + id=3, + summary="Story 3", + description="Description 3", + repo="repo-a", + acceptance_criteria=["Criteria 3"], + ), + ] + + def test_valid_draft_creation(self, valid_items): + """Verify that a valid draft with unique, sequential IDs can be created.""" + now = datetime.now(UTC) + draft = ForgeDecompositionDraft( + parent_key="PROJ-123", + phase="epics", + items=valid_items, + version=1, + created_at=now, + updated_at=now, + ) + assert draft.parent_key == "PROJ-123" + assert draft.phase == "epics" + assert len(draft.items) == 3 + assert draft.version == 1 + assert draft.created_at == now + assert draft.updated_at == now + + def test_valid_draft_with_empty_items(self): + """Verify that a draft with an empty list of items is valid.""" + now = datetime.now(UTC) + draft = ForgeDecompositionDraft( + parent_key="PROJ-123", + phase="tasks", + items=[], + version=2, + created_at=now, + updated_at=now, + ) + assert draft.items == [] + + def test_invalid_phase(self, valid_items): + """Verify that a phase other than 'epics' or 'tasks' raises ValidationError.""" + now = datetime.now(UTC) + with pytest.raises(ValidationError) as exc_info: + ForgeDecompositionDraft( + parent_key="PROJ-123", + phase="invalid", # Invalid phase + items=valid_items, + created_at=now, + updated_at=now, + ) + assert "Input should be 'epics'" in str(exc_info.value) + + def test_duplicate_ids(self): + """Verify that duplicate item IDs raise ValidationError.""" + now = datetime.now(UTC) + items_with_duplicates = [ + DraftItem( + id=1, + summary="Story 1", + description="Description 1", + repo="repo-a", + acceptance_criteria=[], + ), + DraftItem( + id=1, # Duplicate + summary="Story 2", + description="Description 2", + repo="repo-b", + acceptance_criteria=[], + ), + ] + with pytest.raises(ValidationError) as exc_info: + ForgeDecompositionDraft( + parent_key="PROJ-123", + phase="epics", + items=items_with_duplicates, + created_at=now, + updated_at=now, + ) + assert "Draft item IDs must be unique." in str(exc_info.value) + + def test_non_sequential_ids(self): + """Verify that non-sequential item IDs (gaps) raise ValidationError.""" + now = datetime.now(UTC) + items_with_gap = [ + DraftItem( + id=1, + summary="Story 1", + description="D1", + repo="repo-a", + acceptance_criteria=[], + ), + DraftItem( + id=3, # Gap: missing ID 2 + summary="Story 2", + description="D2", + repo="repo-b", + acceptance_criteria=[], + ), + ] + with pytest.raises(ValidationError) as exc_info: + ForgeDecompositionDraft( + parent_key="PROJ-123", + phase="epics", + items=items_with_gap, + created_at=now, + updated_at=now, + ) + assert "Draft item IDs must be sequential starting from 1." in str(exc_info.value) + + def test_sequential_not_starting_from_one(self): + """Verify that IDs that are sequential but do not start from 1 raise ValidationError.""" + now = datetime.now(UTC) + items_not_starting_at_one = [ + DraftItem( + id=2, # Starts at 2 + summary="Story 1", + description="D1", + repo="repo-a", + acceptance_criteria=[], + ), + DraftItem( + id=3, + summary="Story 2", + description="D2", + repo="repo-b", + acceptance_criteria=[], + ), + ] + with pytest.raises(ValidationError) as exc_info: + ForgeDecompositionDraft( + parent_key="PROJ-123", + phase="epics", + items=items_not_starting_at_one, + created_at=now, + updated_at=now, + ) + assert "Draft item IDs must be sequential starting from 1." in str(exc_info.value) + + def test_unordered_but_valid_ids(self): + """Verify that items with IDs that are unique and sequential starting from 1 are valid even if unordered in input.""" + now = datetime.now(UTC) + unordered_items = [ + DraftItem( + id=2, + summary="Story 2", + description="D2", + repo="repo-b", + acceptance_criteria=[], + ), + DraftItem( + id=1, + summary="Story 1", + description="D1", + repo="repo-a", + acceptance_criteria=[], + ), + ] + draft = ForgeDecompositionDraft( + parent_key="PROJ-123", + phase="epics", + items=unordered_items, + created_at=now, + updated_at=now, + ) + assert len(draft.items) == 2 + # Verify the original list order is preserved (or at least valid) + assert draft.items[0].id == 2 + assert draft.items[1].id == 1 + + def test_serialization_and_deserialization(self, valid_items): + """Verify successful JSON serialization and deserialization of the draft model.""" + now = datetime(2025, 1, 1, 12, 0, 0, tzinfo=UTC) + draft = ForgeDecompositionDraft( + parent_key="PROJ-123", + phase="epics", + items=valid_items, + version=1, + created_at=now, + updated_at=now, + ) + + # Serialize to JSON + json_data = draft.model_dump_json() + + # Deserialize back to a new model + restored = ForgeDecompositionDraft.model_validate_json(json_data) + + assert restored.parent_key == draft.parent_key + assert restored.phase == draft.phase + assert restored.version == draft.version + assert restored.created_at == draft.created_at + assert restored.updated_at == draft.updated_at + assert len(restored.items) == len(draft.items) + for original, deserialized in zip(draft.items, restored.items, strict=True): + assert original.id == deserialized.id + assert original.summary == deserialized.summary + assert original.description == deserialized.description + assert original.repo == deserialized.repo + assert original.acceptance_criteria == deserialized.acceptance_criteria diff --git a/tests/unit/orchestrator/gates/test_plan_approval.py b/tests/unit/orchestrator/gates/test_plan_approval.py index 348b06fd5..ed7500ce3 100644 --- a/tests/unit/orchestrator/gates/test_plan_approval.py +++ b/tests/unit/orchestrator/gates/test_plan_approval.py @@ -5,7 +5,7 @@ from forge.models.workflow import TicketType from forge.workflow.feature.state import create_initial_feature_state as create_initial_state -from forge.workflow.gates import plan_approval_gate, route_plan_approval +from forge.workflow.gates import plan_approval_gate, provision_epics, route_plan_approval class TestPlanApprovalGate: @@ -38,6 +38,25 @@ def test_gate_preserves_epic_keys(self, plan_pending_state): assert result["epic_keys"] == ["TEST-124", "TEST-125", "TEST-126"] + def test_gate_pauses_workflow_with_zero_epics_in_non_yolo(self, plan_pending_state): + """In non-YOLO mode, gate pauses even with zero epics.""" + plan_pending_state["epic_keys"] = [] + result = plan_approval_gate(plan_pending_state) + + assert result["is_paused"] is True + assert result["current_node"] == "plan_approval_gate" + + def test_gate_routes_to_retry_with_zero_epics_in_yolo(self, plan_pending_state): + """In YOLO mode, gate routes back to decompose_epics if empty.""" + plan_pending_state["epic_keys"] = [] + plan_pending_state["context"] = {"labels": ["forge:yolo"]} + result = plan_approval_gate(plan_pending_state) + + assert result.get("is_paused") is not True + assert result["current_node"] == "decompose_epics" + assert result["retry_count"] == 1 + assert "No Epics generated" in result["last_error"] + class TestRoutePlanApproval: """Tests for route_plan_approval function.""" @@ -57,15 +76,17 @@ def plan_pending_state(self): state["is_paused"] = True return state - def test_routes_to_tasks_on_approval(self, plan_pending_state): + @pytest.mark.asyncio + async def test_routes_to_tasks_on_approval(self, plan_pending_state): """Approved Plan routes to task generation when not paused.""" plan_pending_state["is_paused"] = False - result = route_plan_approval(plan_pending_state) + result = await route_plan_approval(plan_pending_state) - assert result == "generate_tasks" + assert result == "provision_epics" - def test_routes_to_regenerate_all_on_full_rejection(self, plan_pending_state): + @pytest.mark.asyncio + async def test_routes_to_regenerate_all_on_full_rejection(self, plan_pending_state): """Full plan rejection routes to regenerate all epics.""" plan_pending_state["context"] = { "labels": ["forge:managed", "forge:plan-pending"], @@ -74,11 +95,12 @@ def test_routes_to_regenerate_all_on_full_rejection(self, plan_pending_state): plan_pending_state["feedback_comment"] = "The epic breakdown doesn't make sense." plan_pending_state["revision_requested"] = True - result = route_plan_approval(plan_pending_state) + result = await route_plan_approval(plan_pending_state) assert result == "regenerate_all_epics" - def test_routes_to_update_single_on_epic_rejection(self, plan_pending_state): + @pytest.mark.asyncio + async def test_routes_to_update_single_on_epic_rejection(self, plan_pending_state): """Single epic rejection routes to update that epic.""" plan_pending_state["context"] = { "labels": ["forge:managed", "forge:plan-pending"], @@ -89,17 +111,18 @@ def test_routes_to_update_single_on_epic_rejection(self, plan_pending_state): plan_pending_state["feedback_comment"] = "Epic 2 needs more detail." plan_pending_state["revision_requested"] = True - result = route_plan_approval(plan_pending_state) + result = await route_plan_approval(plan_pending_state) assert result == "update_single_epic" - def test_routes_to_end_when_pending(self, plan_pending_state): + @pytest.mark.asyncio + async def test_routes_to_end_when_pending(self, plan_pending_state): """Pending Plan without feedback routes to END.""" plan_pending_state["context"] = { "labels": ["forge:managed", "forge:plan-pending"], } - result = route_plan_approval(plan_pending_state) + result = await route_plan_approval(plan_pending_state) assert result == END @@ -118,7 +141,8 @@ def state_with_epics(self): state["epic_keys"] = ["TEST-124", "TEST-125", "TEST-126"] return state - def test_full_regen_deletes_all_epics(self, state_with_epics): + @pytest.mark.asyncio + async def test_full_regen_deletes_all_epics(self, state_with_epics): """Full regeneration affects all epics.""" state_with_epics["context"] = { "labels": ["forge:managed", "forge:plan-pending"], @@ -127,11 +151,12 @@ def test_full_regen_deletes_all_epics(self, state_with_epics): state_with_epics["feedback_comment"] = "Start over with a different approach." state_with_epics["revision_requested"] = True - result = route_plan_approval(state_with_epics) + result = await route_plan_approval(state_with_epics) assert result == "regenerate_all_epics" - def test_single_epic_update_preserves_others(self, state_with_epics): + @pytest.mark.asyncio + async def test_single_epic_update_preserves_others(self, state_with_epics): """Single epic update preserves other epics.""" state_with_epics["context"] = { "labels": ["forge:managed", "forge:plan-pending"], @@ -142,14 +167,15 @@ def test_single_epic_update_preserves_others(self, state_with_epics): state_with_epics["feedback_comment"] = "Just fix this one epic." state_with_epics["revision_requested"] = True - result = route_plan_approval(state_with_epics) + result = await route_plan_approval(state_with_epics) assert result == "update_single_epic" # Other epics should remain in state assert "TEST-124" in state_with_epics["epic_keys"] assert "TEST-126" in state_with_epics["epic_keys"] - def test_partial_approval_scenario(self, state_with_epics): + @pytest.mark.asyncio + async def test_partial_approval_scenario(self, state_with_epics): """Some epics approved, one needs revision.""" # This tests the scenario where user approves some epics # but requests changes to one specific epic @@ -163,7 +189,7 @@ def test_partial_approval_scenario(self, state_with_epics): state_with_epics["feedback_comment"] = "Epic 3 scope is too broad." state_with_epics["revision_requested"] = True - result = route_plan_approval(state_with_epics) + result = await route_plan_approval(state_with_epics) assert result == "update_single_epic" @@ -186,41 +212,175 @@ def plan_pending_state(self): state["is_paused"] = False return state - def test_routes_to_answer_question_when_is_question(self, plan_pending_state): + @pytest.mark.asyncio + async def test_routes_to_answer_question_when_is_question(self, plan_pending_state): """Questions route to answer_question node.""" plan_pending_state["is_question"] = True plan_pending_state["feedback_comment"] = "?Why split into two epics?" - result = route_plan_approval(plan_pending_state) + result = await route_plan_approval(plan_pending_state) assert result == "answer_question" - def test_question_takes_priority_over_revision(self, plan_pending_state): + @pytest.mark.asyncio + async def test_question_takes_priority_over_revision(self, plan_pending_state): """Question routing takes priority over revision routing.""" plan_pending_state["is_question"] = True plan_pending_state["revision_requested"] = True plan_pending_state["feedback_comment"] = "?What's the dependency order?" - result = route_plan_approval(plan_pending_state) + result = await route_plan_approval(plan_pending_state) assert result == "answer_question" - def test_routes_to_regenerate_all_when_feedback_not_question(self, plan_pending_state): + @pytest.mark.asyncio + async def test_routes_to_regenerate_all_when_feedback_not_question(self, plan_pending_state): """Normal feedback routes to regenerate all epics.""" plan_pending_state["is_question"] = False plan_pending_state["revision_requested"] = True plan_pending_state["feedback_comment"] = "Rethink the epic breakdown" - result = route_plan_approval(plan_pending_state) + result = await route_plan_approval(plan_pending_state) assert result == "regenerate_all_epics" - def test_question_without_feedback_does_not_route_to_answer(self, plan_pending_state): + @pytest.mark.asyncio + async def test_question_without_feedback_does_not_route_to_answer(self, plan_pending_state): """is_question alone without feedback_comment doesn't route to answer.""" plan_pending_state["is_question"] = True plan_pending_state["feedback_comment"] = "" - result = route_plan_approval(plan_pending_state) + result = await route_plan_approval(plan_pending_state) + + # Should proceed to provision_epics since not paused + assert result == "provision_epics" + + +class TestPlanDraftProvisioning: + """Tests for draft-based ticket provisioning in provision_epics.""" + + @pytest.fixture + def approved_plan_state(self): + """Approved plan state waiting for ticket creation.""" + state = create_initial_state( + thread_id="test-thread", + ticket_key="TEST-123", + ticket_type=TicketType.FEATURE, + ) + state["is_paused"] = False + state["epic_keys"] = [] + return state + + @pytest.mark.asyncio + async def test_successful_draft_provisioning(self, approved_plan_state): + """Verify successful download, parsing, skipping excluded items, and deletion on success.""" + from datetime import UTC, datetime + from unittest.mock import AsyncMock, patch + + from forge.models.draft import DraftItem, ForgeDecompositionDraft + + draft_item_1 = DraftItem( + id=1, + summary="Epic One", + description="Details of epic 1", + repo="org/repo-1", + acceptance_criteria=[], + excluded=False, + ) + draft_item_2 = DraftItem( + id=2, + summary="Epic Two", + description="Details of epic 2", + repo="org/repo-2", + acceptance_criteria=[], + excluded=True, # Excluded! + ) + draft = ForgeDecompositionDraft( + parent_key="TEST-123", + phase="epics", + items=[draft_item_1, draft_item_2], + version=1, + created_at=datetime.now(UTC), + updated_at=datetime.now(UTC), + ) + + with ( + patch("forge.integrations.jira.client.JiraClient") as MockJira, + patch("forge.workflow.utils.draft_manager.DraftManager") as MockDraftManager, + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + + mock_issue = AsyncMock() + mock_issue.project_key = "TEST" + mock_jira.get_issue = AsyncMock(return_value=mock_issue) + mock_jira.create_epic = AsyncMock(return_value="EPIC-101") + mock_jira.search_issues = AsyncMock(return_value=[]) # Idempotency guard finds nothing + + MockDraftManager.get_draft_attachment = AsyncMock(return_value=draft) + MockDraftManager.delete_draft_attachment = AsyncMock() + + approved_plan_state["plan_draft"] = draft + result = await provision_epics(approved_plan_state) + + assert result["epic_keys"] == ["EPIC-101"] + + # Verify creations and exclusions + mock_jira.create_epic.assert_called_once_with( + project_key="TEST", + summary="Epic One", + description="Details of epic 1", + parent_key="TEST-123", + labels=["forge:managed", "forge:parent:TEST-123", "repo:org/repo-1"], + ) + + # Verify draft deleted + MockDraftManager.delete_draft_attachment.assert_called_once() + + @pytest.mark.asyncio + async def test_retains_draft_on_failure(self, approved_plan_state): + """Verify that draft attachment is not deleted if epic creation fails midway.""" + from datetime import UTC, datetime + from unittest.mock import AsyncMock, patch + + from forge.models.draft import DraftItem, ForgeDecompositionDraft + + draft_item_1 = DraftItem( + id=1, + summary="Epic One", + description="Details of epic 1", + repo="org/repo-1", + acceptance_criteria=[], + excluded=False, + ) + draft = ForgeDecompositionDraft( + parent_key="TEST-123", + phase="epics", + items=[draft_item_1], + version=1, + created_at=datetime.now(UTC), + updated_at=datetime.now(UTC), + ) + + with ( + patch("forge.integrations.jira.client.JiraClient") as MockJira, + patch("forge.workflow.utils.draft_manager.DraftManager") as MockDraftManager, + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + + mock_issue = AsyncMock() + mock_issue.project_key = "TEST" + mock_jira.get_issue = AsyncMock(return_value=mock_issue) + mock_jira.create_epic = AsyncMock(side_effect=Exception("Jira failure midway!")) + mock_jira.search_issues = AsyncMock(return_value=[]) # Idempotency guard finds nothing + + MockDraftManager.get_draft_attachment = AsyncMock(return_value=draft) + MockDraftManager.delete_draft_attachment = AsyncMock() + + approved_plan_state["plan_draft"] = draft + with pytest.raises(Exception, match="Jira failure midway!"): + await provision_epics(approved_plan_state) - # Should proceed to generate_tasks since not paused - assert result == "generate_tasks" + # Deletion should not have been called + MockDraftManager.delete_draft_attachment.assert_not_called() diff --git a/tests/unit/orchestrator/gates/test_task_approval.py b/tests/unit/orchestrator/gates/test_task_approval.py index 5e8b8f7c8..2938daa49 100644 --- a/tests/unit/orchestrator/gates/test_task_approval.py +++ b/tests/unit/orchestrator/gates/test_task_approval.py @@ -5,7 +5,7 @@ from forge.models.workflow import TicketType from forge.workflow.feature.state import create_initial_feature_state as create_initial_state -from forge.workflow.gates import route_task_approval, task_approval_gate +from forge.workflow.gates import provision_tasks, route_task_approval, task_approval_gate class TestTaskApprovalGate: @@ -39,6 +39,25 @@ def test_gate_preserves_task_keys(self, task_pending_state): assert result["task_keys"] == ["TEST-130", "TEST-131", "TEST-132"] + def test_gate_pauses_workflow_with_zero_tasks_in_non_yolo(self, task_pending_state): + """In non-YOLO mode, gate pauses even with zero tasks.""" + task_pending_state["task_keys"] = [] + result = task_approval_gate(task_pending_state) + + assert result["is_paused"] is True + assert result["current_node"] == "task_approval_gate" + + def test_gate_routes_to_retry_with_zero_tasks_in_yolo(self, task_pending_state): + """In YOLO mode, gate routes back to generate_tasks if empty.""" + task_pending_state["task_keys"] = [] + task_pending_state["context"] = {"labels": ["forge:yolo"]} + result = task_approval_gate(task_pending_state) + + assert result.get("is_paused") is not True + assert result["current_node"] == "generate_tasks" + assert result["retry_count"] == 1 + assert "No Tasks generated" in result["last_error"] + class TestRouteTaskApproval: """Tests for route_task_approval function.""" @@ -59,54 +78,60 @@ def task_pending_state(self): state["is_paused"] = True return state - def test_routes_to_task_router_on_approval(self, task_pending_state): + @pytest.mark.asyncio + async def test_routes_to_task_router_on_approval(self, task_pending_state): """Approved Tasks routes to task router when not paused.""" task_pending_state["is_paused"] = False - result = route_task_approval(task_pending_state) + result = await route_task_approval(task_pending_state) - assert result == "task_router" + assert result == "provision_tasks" - def test_routes_to_regenerate_all_on_feature_rejection(self, task_pending_state): + @pytest.mark.asyncio + async def test_routes_to_regenerate_all_on_feature_rejection(self, task_pending_state): """Full task rejection routes to regenerate all tasks.""" task_pending_state["feedback_comment"] = "The task breakdown is too coarse." task_pending_state["revision_requested"] = True - result = route_task_approval(task_pending_state) + result = await route_task_approval(task_pending_state) assert result == "regenerate_all_tasks" - def test_routes_to_update_single_on_task_rejection(self, task_pending_state): + @pytest.mark.asyncio + async def test_routes_to_update_single_on_task_rejection(self, task_pending_state): """Single task rejection routes to update that task.""" task_pending_state["current_task_key"] = "TEST-131" task_pending_state["feedback_comment"] = "Task 2 needs more detail." task_pending_state["revision_requested"] = True - result = route_task_approval(task_pending_state) + result = await route_task_approval(task_pending_state) assert result == "update_single_task" - def test_routes_to_regenerate_all_on_epic_sourced_rejection(self, task_pending_state): + @pytest.mark.asyncio + async def test_routes_to_regenerate_all_on_epic_sourced_rejection(self, task_pending_state): """Epic-sourced task feedback routes to regenerate_epic_tasks, not all tasks.""" task_pending_state["current_epic_key"] = "TEST-124" task_pending_state["feedback_comment"] = "Revise the tasks for this epic." task_pending_state["revision_requested"] = True - result = route_task_approval(task_pending_state) + result = await route_task_approval(task_pending_state) assert result == "regenerate_epic_tasks" - def test_feature_level_rejection_still_regenerates_all(self, task_pending_state): + @pytest.mark.asyncio + async def test_feature_level_rejection_still_regenerates_all(self, task_pending_state): """Feature-level feedback (no epic key) still routes to regenerate_all_tasks.""" task_pending_state["current_epic_key"] = None task_pending_state["feedback_comment"] = "The whole task breakdown is wrong." task_pending_state["revision_requested"] = True - result = route_task_approval(task_pending_state) + result = await route_task_approval(task_pending_state) assert result == "regenerate_all_tasks" - def test_epic_rejection_with_empty_body_routes_to_regenerate_epic_tasks( + @pytest.mark.asyncio + async def test_epic_rejection_with_empty_body_routes_to_regenerate_epic_tasks( self, task_pending_state ): """Empty-body '!' on an Epic must not fall through to task_router (approval).""" @@ -114,13 +139,14 @@ def test_epic_rejection_with_empty_body_routes_to_regenerate_epic_tasks( task_pending_state["feedback_comment"] = "" task_pending_state["revision_requested"] = True - result = route_task_approval(task_pending_state) + result = await route_task_approval(task_pending_state) assert result == "regenerate_epic_tasks" - def test_routes_to_end_when_pending(self, task_pending_state): + @pytest.mark.asyncio + async def test_routes_to_end_when_pending(self, task_pending_state): """Pending Tasks without feedback routes to END.""" - result = route_task_approval(task_pending_state) + result = await route_task_approval(task_pending_state) assert result == END @@ -144,41 +170,180 @@ def task_pending_state(self): state["is_paused"] = False return state - def test_routes_to_answer_question_when_is_question(self, task_pending_state): + @pytest.mark.asyncio + async def test_routes_to_answer_question_when_is_question(self, task_pending_state): """Questions route to answer_question node.""" task_pending_state["is_question"] = True task_pending_state["feedback_comment"] = "?Why are there two tasks for this?" - result = route_task_approval(task_pending_state) + result = await route_task_approval(task_pending_state) assert result == "answer_question" - def test_question_takes_priority_over_revision(self, task_pending_state): + @pytest.mark.asyncio + async def test_question_takes_priority_over_revision(self, task_pending_state): """Question routing takes priority over revision routing.""" task_pending_state["is_question"] = True task_pending_state["revision_requested"] = True task_pending_state["feedback_comment"] = "?What's the testing strategy?" - result = route_task_approval(task_pending_state) + result = await route_task_approval(task_pending_state) assert result == "answer_question" - def test_routes_to_regenerate_when_feedback_not_question(self, task_pending_state): + @pytest.mark.asyncio + async def test_routes_to_regenerate_when_feedback_not_question(self, task_pending_state): """Normal feedback routes to regenerate all tasks.""" task_pending_state["is_question"] = False task_pending_state["revision_requested"] = True task_pending_state["feedback_comment"] = "Add more tasks for testing" - result = route_task_approval(task_pending_state) + result = await route_task_approval(task_pending_state) assert result == "regenerate_all_tasks" - def test_question_without_feedback_does_not_route_to_answer(self, task_pending_state): + @pytest.mark.asyncio + async def test_question_without_feedback_does_not_route_to_answer(self, task_pending_state): """is_question alone without feedback_comment doesn't route to answer.""" task_pending_state["is_question"] = True task_pending_state["feedback_comment"] = "" - result = route_task_approval(task_pending_state) + result = await route_task_approval(task_pending_state) + + # Should proceed to provision_tasks since not paused + assert result == "provision_tasks" + + +class TestTaskDraftProvisioning: + """Tests for draft-based ticket provisioning in provision_tasks.""" + + @pytest.fixture + def approved_task_state(self): + """Approved task state waiting for ticket creation.""" + state = create_initial_state( + thread_id="test-thread", + ticket_key="TEST-123", + ticket_type=TicketType.FEATURE, + ) + state["is_paused"] = False + state["epic_keys"] = ["EPIC-124"] + state["task_keys"] = [] + return state + + @pytest.mark.asyncio + async def test_successful_draft_provisioning(self, approved_task_state): + """Verify successful download, parsing, skipping excluded tasks, and deletion on success.""" + from datetime import UTC, datetime + from unittest.mock import AsyncMock, patch + + from forge.models.draft import DraftItem, ForgeDecompositionDraft + + draft_item_1 = DraftItem( + id=1, + summary="Task One", + description="Details of task 1", + repo="org/repo-1", + acceptance_criteria=[], + excluded=False, + epic_key="EPIC-124", + ) + draft_item_2 = DraftItem( + id=2, + summary="Task Two", + description="Details of task 2", + repo="org/repo-2", + acceptance_criteria=[], + excluded=True, # Excluded! + epic_key="EPIC-124", + ) + draft = ForgeDecompositionDraft( + parent_key="TEST-123", + phase="tasks", + items=[draft_item_1, draft_item_2], + version=1, + created_at=datetime.now(UTC), + updated_at=datetime.now(UTC), + ) + + with ( + patch("forge.integrations.jira.client.JiraClient") as MockJira, + patch("forge.workflow.utils.draft_manager.DraftManager") as MockDraftManager, + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + + mock_issue = AsyncMock() + mock_issue.project_key = "TEST" + mock_jira.get_issue = AsyncMock(return_value=mock_issue) + mock_jira.create_task = AsyncMock(return_value="TASK-201") + mock_jira.search_issues = AsyncMock(return_value=[]) # Idempotency guard finds nothing + + MockDraftManager.get_draft_attachment = AsyncMock(return_value=draft) + MockDraftManager.delete_draft_attachment = AsyncMock() + + approved_task_state["tasks_draft"] = draft + result = await provision_tasks(approved_task_state) + + assert result["task_keys"] == ["TASK-201"] + assert result["tasks_by_repo"] == {"org/repo-1": ["TASK-201"]} + + # Verify creations and exclusions + mock_jira.create_task.assert_called_once_with( + project_key="TEST", + summary="Task One", + description="Details of task 1", + parent_key="EPIC-124", + labels=["forge:managed", "forge:parent:TEST-123", "repo:org/repo-1"], + ) + + # Task drafts live only in workflow state; no attachment cleanup occurs. + MockDraftManager.delete_draft_attachment.assert_not_called() + + @pytest.mark.asyncio + async def test_retains_draft_on_failure(self, approved_task_state): + """Verify that draft attachment is not deleted if task creation fails midway.""" + from datetime import UTC, datetime + from unittest.mock import AsyncMock, patch + + from forge.models.draft import DraftItem, ForgeDecompositionDraft + + draft_item_1 = DraftItem( + id=1, + summary="Task One", + description="Details of task 1", + repo="org/repo-1", + acceptance_criteria=[], + excluded=False, + epic_key="EPIC-124", + ) + draft = ForgeDecompositionDraft( + parent_key="TEST-123", + phase="tasks", + items=[draft_item_1], + version=1, + created_at=datetime.now(UTC), + updated_at=datetime.now(UTC), + ) + + with ( + patch("forge.integrations.jira.client.JiraClient") as MockJira, + patch("forge.workflow.utils.draft_manager.DraftManager") as MockDraftManager, + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + + mock_issue = AsyncMock() + mock_issue.project_key = "TEST" + mock_jira.get_issue = AsyncMock(return_value=mock_issue) + mock_jira.create_task = AsyncMock(side_effect=Exception("Jira task failure!")) + mock_jira.search_issues = AsyncMock(return_value=[]) # Idempotency guard finds nothing + + MockDraftManager.get_draft_attachment = AsyncMock(return_value=draft) + MockDraftManager.delete_draft_attachment = AsyncMock() + + approved_task_state["tasks_draft"] = draft + with pytest.raises(Exception, match="Jira task failure!"): + await provision_tasks(approved_task_state) - # Should proceed to task_router since not paused - assert result == "task_router" + # Deletion should not have been called + MockDraftManager.delete_draft_attachment.assert_not_called() diff --git a/tests/unit/sandbox/test_container_entrypoint_git_commit.py b/tests/unit/sandbox/test_container_entrypoint_git_commit.py index ba1067928..712eef67c 100644 --- a/tests/unit/sandbox/test_container_entrypoint_git_commit.py +++ b/tests/unit/sandbox/test_container_entrypoint_git_commit.py @@ -7,6 +7,11 @@ def _load_entrypoint_module(): module_path = Path(__file__).parents[3] / "containers" / "entrypoint.py" + import sys + + containers_dir = str(module_path.parent) + if containers_dir not in sys.path: + sys.path.insert(0, containers_dir) spec = importlib.util.spec_from_file_location("forge_container_entrypoint", module_path) assert spec is not None module = importlib.util.module_from_spec(spec) diff --git a/tests/unit/workflow/nodes/test_epic_decomposition.py b/tests/unit/workflow/nodes/test_epic_decomposition.py index 8786542c6..ab2fe2402 100644 --- a/tests/unit/workflow/nodes/test_epic_decomposition.py +++ b/tests/unit/workflow/nodes/test_epic_decomposition.py @@ -7,6 +7,7 @@ from forge.integrations.jira.client import MissingProjectConfig from forge.models.workflow import ForgeLabel from forge.workflow.nodes.epic_decomposition import decompose_epics, regenerate_all_epics +from forge.workflow.utils.draft_manager import DraftManager @pytest.fixture @@ -17,6 +18,7 @@ def base_state(): "qa_history": [], "generation_context": {}, "retry_count": 0, + "yolo_mode": True, } @@ -115,7 +117,9 @@ async def test_blocks_and_comments_when_forge_repos_missing(self, base_state, mo patch("forge.workflow.nodes.epic_decomposition.JiraClient") as MockJira, patch("forge.workflow.nodes.epic_decomposition.ForgeAgent") as MockAgent, patch("forge.workflow.nodes.epic_decomposition.post_qa_summary_if_needed"), - patch("forge.workflow.nodes.epic_decomposition.get_settings", return_value=mock_settings), + patch( + "forge.workflow.nodes.epic_decomposition.get_settings", return_value=mock_settings + ), ): mock_jira = AsyncMock() MockJira.return_value = mock_jira @@ -136,9 +140,7 @@ async def test_blocks_and_comments_when_forge_repos_missing(self, base_state, mo assert "forge.repos" in comment_text assert "forge:retry" in comment_text - mock_jira.set_workflow_label.assert_called_once_with( - "MYPROJ-1", ForgeLabel.BLOCKED - ) + mock_jira.set_workflow_label.assert_called_once_with("MYPROJ-1", ForgeLabel.BLOCKED) assert result["last_error"] assert result["current_node"] == "decompose_epics" @@ -153,7 +155,9 @@ async def test_blocks_and_comments_when_forge_repos_malformed(self, base_state, patch("forge.workflow.nodes.epic_decomposition.JiraClient") as MockJira, patch("forge.workflow.nodes.epic_decomposition.ForgeAgent") as MockAgent, patch("forge.workflow.nodes.epic_decomposition.post_qa_summary_if_needed"), - patch("forge.workflow.nodes.epic_decomposition.get_settings", return_value=mock_settings), + patch( + "forge.workflow.nodes.epic_decomposition.get_settings", return_value=mock_settings + ), ): mock_jira = AsyncMock() MockJira.return_value = mock_jira @@ -171,9 +175,7 @@ async def test_blocks_and_comments_when_forge_repos_malformed(self, base_state, result = await decompose_epics(base_state) - mock_jira.set_workflow_label.assert_called_once_with( - "MYPROJ-1", ForgeLabel.BLOCKED - ) + mock_jira.set_workflow_label.assert_called_once_with("MYPROJ-1", ForgeLabel.BLOCKED) assert result["last_error"] @@ -255,3 +257,177 @@ async def test_regenerate_all_epics_clears_revision_flags_after_new_epics( assert result["current_node"] == "plan_approval_gate" assert result["revision_requested"] is False assert result["feedback_comment"] is None + + +class TestDecomposeEpicsDraftReview: + """Tests for the non-YOLO draft review gate flow in decompose_epics.""" + + @pytest.mark.asyncio + async def test_decompose_epics_draft_review_flow_success( + self, base_state, mock_issue, mock_epics_data + ): + """When yolo_mode is False, decomposes epics into a draft JSON, deletes old attachments, saves new one, posts comment, and pauses.""" + state = {**base_state, "yolo_mode": False} + + with ( + patch("forge.workflow.nodes.epic_decomposition.JiraClient") as MockJira, + patch("forge.workflow.nodes.epic_decomposition.ForgeAgent") as MockAgent, + patch("forge.workflow.nodes.epic_decomposition.DraftManager") as MockDraftManager, + patch("forge.workflow.nodes.epic_decomposition.post_qa_summary_if_needed"), + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + mock_jira.get_issue = AsyncMock(return_value=mock_issue) + mock_jira.get_labels = AsyncMock(return_value=[]) + mock_jira.get_project_repos = AsyncMock(return_value=["acme/backend"]) + mock_jira.set_workflow_label = AsyncMock() + mock_jira.add_comment = AsyncMock() + + mock_agent = AsyncMock() + MockAgent.return_value = mock_agent + mock_agent.generate_epics = AsyncMock(return_value=mock_epics_data) + + MockDraftManager.delete_draft_attachment = AsyncMock() + MockDraftManager.save_draft_attachment = AsyncMock() + MockDraftManager.format_review_comment.side_effect = DraftManager.format_review_comment + + result = await decompose_epics(state) + + # 1. Verify saved_draft is stored in the result state + saved_draft = result["plan_draft"] + assert saved_draft is not None + assert saved_draft.parent_key == "MYPROJ-1" + assert saved_draft.phase == "epics" + assert len(saved_draft.items) == 1 + assert saved_draft.items[0].summary == "Epic One" + assert saved_draft.items[0].description == "Do stuff." + assert saved_draft.items[0].repo == "acme/backend" + + # 3. Verify formatted comment posted + assert mock_jira.add_comment.call_count == 2 + comment_text = mock_jira.add_comment.call_args_list[1][0][1] + assert "### 📋 Proposed Epics Draft" in comment_text + assert "Epic One" in comment_text + assert "acme/backend" in comment_text + + # 4. Verify workflow label updated to PLAN_PENDING + mock_jira.set_workflow_label.assert_called_once_with("MYPROJ-1", ForgeLabel.PLAN_PENDING) + + # 5. Verify state transitions to plan_approval_gate and pauses + assert result["current_node"] == "plan_approval_gate" + assert result["is_paused"] is True + assert result["epic_keys"] == [] + + @pytest.mark.asyncio + async def test_decompose_epics_draft_review_truncation_limits(self, base_state, mock_issue): + """When the item list has > 15 elements, comment falls back to a condensed table.""" + state = {**base_state, "yolo_mode": False} + + # Mock 16 items + many_epics_data = [ + {"summary": f"Epic {i}", "plan": f"Plan {i}", "repo": f"repo-{i}"} for i in range(1, 17) + ] + + with ( + patch("forge.workflow.nodes.epic_decomposition.JiraClient") as MockJira, + patch("forge.workflow.nodes.epic_decomposition.ForgeAgent") as MockAgent, + patch("forge.workflow.nodes.epic_decomposition.DraftManager") as MockDraftManager, + patch("forge.workflow.nodes.epic_decomposition.post_qa_summary_if_needed"), + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + mock_jira.get_issue = AsyncMock(return_value=mock_issue) + mock_jira.get_labels = AsyncMock(return_value=[]) + mock_jira.get_project_repos = AsyncMock(return_value=["acme/backend"]) + mock_jira.set_workflow_label = AsyncMock() + mock_jira.add_comment = AsyncMock() + + mock_agent = AsyncMock() + MockAgent.return_value = mock_agent + mock_agent.generate_epics = AsyncMock(return_value=many_epics_data) + + MockDraftManager.delete_draft_attachment = AsyncMock() + MockDraftManager.save_draft_attachment = AsyncMock() + MockDraftManager.format_review_comment.side_effect = DraftManager.format_review_comment + + await decompose_epics(state) + + # Verify comment is in condensed table format + assert mock_jira.add_comment.call_count == 2 + comment_text = mock_jira.add_comment.call_args_list[1][0][1] + assert "### 📋 Proposed Epics Draft (Condensed)" in comment_text + assert "Warning" in comment_text + assert "forge-epics-draft.json" in comment_text + # Condensed table should only show IDs, summaries, and target repos + # Detailed descriptions/plans (like Plan 1) should NOT be in the comment + assert "Plan 1" not in comment_text + assert "Epic 1" in comment_text + assert "repo-1" in comment_text + + @pytest.mark.asyncio + async def test_decompose_epics_draft_review_truncation_characters(self, base_state, mock_issue): + """When comment exceeds 32,767 characters, comment falls back to a condensed table.""" + state = {**base_state, "yolo_mode": False} + + # Mock huge description to exceed character limit + huge_epics_data = [{"summary": "Epic One", "plan": "A" * 35000, "repo": "acme/backend"}] + + with ( + patch("forge.workflow.nodes.epic_decomposition.JiraClient") as MockJira, + patch("forge.workflow.nodes.epic_decomposition.ForgeAgent") as MockAgent, + patch("forge.workflow.nodes.epic_decomposition.DraftManager") as MockDraftManager, + patch("forge.workflow.nodes.epic_decomposition.post_qa_summary_if_needed"), + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + mock_jira.get_issue = AsyncMock(return_value=mock_issue) + mock_jira.get_labels = AsyncMock(return_value=[]) + mock_jira.get_project_repos = AsyncMock(return_value=["acme/backend"]) + mock_jira.set_workflow_label = AsyncMock() + mock_jira.add_comment = AsyncMock() + + mock_agent = AsyncMock() + MockAgent.return_value = mock_agent + mock_agent.generate_epics = AsyncMock(return_value=huge_epics_data) + + MockDraftManager.delete_draft_attachment = AsyncMock() + MockDraftManager.save_draft_attachment = AsyncMock() + MockDraftManager.format_review_comment.side_effect = DraftManager.format_review_comment + + await decompose_epics(state) + + # Verify comment is in condensed table format due to length + assert mock_jira.add_comment.call_count == 2 + comment_text = mock_jira.add_comment.call_args_list[1][0][1] + assert "### 📋 Proposed Epics Draft (Condensed)" in comment_text + assert "Warning" in comment_text + assert "forge-epics-draft.json" in comment_text + assert "A" * 35000 not in comment_text + assert "Epic One" in comment_text + assert "acme/backend" in comment_text + + @pytest.mark.asyncio + async def test_decompose_epics_empty_data_retry(self, base_state, mock_issue): + """When empty epics_data is returned, returns a retry state with last_error and retry_count incremented.""" + state = {**base_state} + + with ( + patch("forge.workflow.nodes.epic_decomposition.JiraClient") as MockJira, + patch("forge.workflow.nodes.epic_decomposition.ForgeAgent") as MockAgent, + patch("forge.workflow.nodes.epic_decomposition.post_qa_summary_if_needed"), + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + mock_jira.get_issue = AsyncMock(return_value=mock_issue) + mock_jira.get_labels = AsyncMock(return_value=[]) + mock_jira.get_project_repos = AsyncMock(return_value=["acme/backend"]) + + mock_agent = AsyncMock() + MockAgent.return_value = mock_agent + mock_agent.generate_epics = AsyncMock(return_value=[]) + + result = await decompose_epics(state) + + assert result["current_node"] == "decompose_epics" + assert result["retry_count"] == 1 + assert "Epic generation returned no results" in result["last_error"] diff --git a/tests/unit/workflow/nodes/test_implementation_status_instrumentation.py b/tests/unit/workflow/nodes/test_implementation_status_instrumentation.py index 20b03a98e..120105f6e 100644 --- a/tests/unit/workflow/nodes/test_implementation_status_instrumentation.py +++ b/tests/unit/workflow/nodes/test_implementation_status_instrumentation.py @@ -152,9 +152,10 @@ async def test_post_status_comment_called_at_completion_on_success(self): with ( patch("forge.workflow.nodes.implementation.JiraClient", return_value=mock_jira), patch("forge.workflow.nodes.implementation.ContainerRunner", return_value=mock_runner), - patch("forge.workflow.nodes.implementation.post_status_comment") as mock_post_status, + patch( + "forge.workflow.nodes.implementation.post_status_comment", new_callable=AsyncMock + ) as mock_post_status, ): - mock_post_status.return_value = AsyncMock() result = await implement_task(state) # Verify post_status_comment was called twice (start + completion) @@ -186,9 +187,10 @@ async def test_post_status_comment_not_called_at_completion_on_failure(self): with ( patch("forge.workflow.nodes.implementation.JiraClient", return_value=mock_jira), patch("forge.workflow.nodes.implementation.ContainerRunner", return_value=mock_runner), - patch("forge.workflow.nodes.implementation.post_status_comment") as mock_post_status, + patch( + "forge.workflow.nodes.implementation.post_status_comment", new_callable=AsyncMock + ) as mock_post_status, ): - mock_post_status.return_value = AsyncMock() result = await implement_task(state) # Verify post_status_comment was called only once (start, NOT completion) diff --git a/tests/unit/workflow/nodes/test_task_generation.py b/tests/unit/workflow/nodes/test_task_generation.py index 54de77d6e..55e71340c 100644 --- a/tests/unit/workflow/nodes/test_task_generation.py +++ b/tests/unit/workflow/nodes/test_task_generation.py @@ -5,6 +5,7 @@ import pytest from forge.integrations.jira.models import JiraIssue +from forge.models.workflow import ForgeLabel from forge.workflow.nodes.task_generation import ( _generate_tasks_for_epic, _parse_tasks_response, @@ -12,6 +13,7 @@ regenerate_all_tasks, regenerate_epic_tasks, ) +from forge.workflow.utils.draft_manager import DraftManager @pytest.fixture @@ -24,23 +26,36 @@ def base_state(): "task_keys": [], "tasks_by_repo": {}, "retry_count": 0, + "yolo_mode": True, } @pytest.fixture def mock_parent_issue(): - issue = MagicMock() - issue.project_key = "MYPROJ" - issue.summary = "Feature summary" - return issue + from forge.integrations.jira.models import JiraIssue + + return JiraIssue( + key="MYPROJ-123", + id="10123", + summary="Feature summary", + description="", + status="In Progress", + issue_type="Feature", + ) @pytest.fixture def mock_epic_issue(): - issue = MagicMock() - issue.summary = "Epic summary" - issue.description = "Implement the backend pieces." - return issue + from forge.integrations.jira.models import JiraIssue + + return JiraIssue( + key="MYPROJ-124", + id="10124", + summary="Epic summary", + description="Implement the backend pieces.", + status="In Progress", + issue_type="Epic", + ) @pytest.fixture @@ -585,3 +600,175 @@ async def test_orphaned_task_with_none_parent_logged_as_warning(self, base_state r for r in caplog.records if "TASK-100" in r.message and "parent" in r.message.lower() ] assert orphan_warnings, "Expected a warning about the orphaned task TASK-100" + + +class TestTaskGenerationDraftReview: + """Tests for the non-YOLO draft review gate flow in generate_tasks.""" + + @pytest.mark.asyncio + async def test_generate_tasks_draft_review_flow_success( + self, base_state, mock_parent_issue, mock_epic_issue, mock_tasks_data + ): + """When yolo_mode is False, generates tasks into a draft JSON, deletes old attachments, saves new one, posts comment, and pauses.""" + state = {**base_state, "yolo_mode": False} + + with ( + patch("forge.workflow.nodes.task_generation.JiraClient") as MockJira, + patch("forge.workflow.nodes.task_generation.ForgeAgent") as MockAgent, + patch("forge.workflow.nodes.task_generation.DraftManager") as MockDraftManager, + patch("forge.workflow.nodes.task_generation.post_status_comment"), + patch( + "forge.workflow.nodes.task_generation._generate_tasks_for_epic", + new_callable=AsyncMock, + return_value=mock_tasks_data, + ), + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + mock_jira.get_issue = AsyncMock(side_effect=[mock_parent_issue, mock_epic_issue]) + mock_jira.get_labels = AsyncMock(return_value=[]) + mock_jira.set_workflow_label = AsyncMock() + mock_jira.add_comment = AsyncMock() + + mock_agent = AsyncMock() + MockAgent.return_value = mock_agent + + MockDraftManager.delete_draft_attachment = AsyncMock() + MockDraftManager.save_task_draft_with_slices = AsyncMock() + MockDraftManager.post_task_draft_review = AsyncMock() + MockDraftManager.format_review_comment.side_effect = DraftManager.format_review_comment + + result = await generate_tasks(state) + + # 1. Verify tasks_draft is saved in state + saved_draft = result["tasks_draft"] + assert saved_draft is not None + assert saved_draft.phase == "tasks" + assert len(saved_draft.items) == 1 + assert saved_draft.items[0].summary == "Task One" + assert saved_draft.items[0].description == "Do the first thing." + assert saved_draft.items[0].repo == "acme/backend" + assert saved_draft.items[0].epic_key == "MYPROJ-10" + + # 2. Verify DraftManager posted the review + MockDraftManager.post_task_draft_review.assert_called_once_with( + mock_jira, "MYPROJ-1", saved_draft + ) + + # 4. Verify workflow label updated to TASK_PENDING + mock_jira.set_workflow_label.assert_called_once_with("MYPROJ-1", ForgeLabel.TASK_PENDING) + + # 5. Verify state transitions to task_approval_gate and pauses + assert result["current_node"] == "task_approval_gate" + assert result["is_paused"] is True + assert result["task_keys"] == [] + + @pytest.mark.asyncio + async def test_generate_tasks_draft_review_truncation_limits( + self, base_state, mock_parent_issue, mock_epic_issue + ): + """More than 15 tasks remain fully visible without an attachment.""" + state = {**base_state, "yolo_mode": False} + + # Mock 16 items + many_tasks_data = [ + {"summary": f"Task {i}", "description": f"Desc {i}", "repo": f"acme/repo-{i}"} + for i in range(1, 17) + ] + + with ( + patch("forge.workflow.nodes.task_generation.JiraClient") as MockJira, + patch("forge.workflow.nodes.task_generation.ForgeAgent") as MockAgent, + patch("forge.workflow.nodes.task_generation.DraftManager") as MockDraftManager, + patch("forge.workflow.nodes.task_generation.post_status_comment"), + patch( + "forge.workflow.nodes.task_generation._generate_tasks_for_epic", + new_callable=AsyncMock, + return_value=many_tasks_data, + ), + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + mock_jira.get_issue = AsyncMock(side_effect=[mock_parent_issue, mock_epic_issue]) + mock_jira.get_labels = AsyncMock(return_value=[]) + mock_jira.set_workflow_label = AsyncMock() + mock_jira.add_comment = AsyncMock() + + mock_agent = AsyncMock() + MockAgent.return_value = mock_agent + + MockDraftManager.delete_draft_attachment = AsyncMock() + MockDraftManager.save_task_draft_with_slices = AsyncMock() + MockDraftManager.post_task_draft_review.side_effect = ( + DraftManager.post_task_draft_review + ) + MockDraftManager.format_review_comment.side_effect = DraftManager.format_review_comment + + await generate_tasks(state) + + # All task details stay in the Epic comment; no attachment fallback is needed. + assert mock_jira.add_comment.call_count == 2 + comment_text = mock_jira.add_comment.call_args_list[0][0][1] + assert "### 📋 Proposed Tasks Draft (Condensed)" not in comment_text + assert "forge-tasks-draft.json" not in comment_text + assert "Desc 1" in comment_text + assert "Desc 16" in comment_text + assert "Task 1" in comment_text + assert "acme/repo-1" in comment_text + + @pytest.mark.asyncio + async def test_generate_tasks_draft_review_truncation_characters( + self, base_state, mock_parent_issue, mock_epic_issue + ): + """When comment exceeds 32,767 characters, comment falls back to a condensed table.""" + state = {**base_state, "yolo_mode": False} + + # Mock huge description to exceed character limit + huge_tasks_data = [ + {"summary": "Task One", "description": "A" * 35000, "repo": "acme/backend"} + ] + + with ( + patch("forge.workflow.nodes.task_generation.JiraClient") as MockJira, + patch("forge.workflow.nodes.task_generation.ForgeAgent") as MockAgent, + patch("forge.workflow.nodes.task_generation.DraftManager") as MockDraftManager, + patch("forge.workflow.nodes.task_generation.post_status_comment"), + patch( + "forge.workflow.nodes.task_generation._generate_tasks_for_epic", + new_callable=AsyncMock, + return_value=huge_tasks_data, + ), + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + mock_jira.get_issue = AsyncMock(side_effect=[mock_parent_issue, mock_epic_issue]) + mock_jira.get_labels = AsyncMock(return_value=[]) + mock_jira.set_workflow_label = AsyncMock() + mock_jira.add_comment = AsyncMock() + + mock_agent = AsyncMock() + MockAgent.return_value = mock_agent + + MockDraftManager.delete_draft_attachment = AsyncMock() + MockDraftManager.save_task_draft_with_slices = AsyncMock() + MockDraftManager.post_task_draft_review.side_effect = ( + DraftManager.post_task_draft_review + ) + MockDraftManager.format_review_comment.side_effect = DraftManager.format_review_comment + + await generate_tasks(state) + + # The oversized task review is split across Epic continuation comments, + # followed by the Feature navigation comment. + assert mock_jira.add_comment.call_count > 2 + epic_comments = [ + call.args[1] + for call in mock_jira.add_comment.call_args_list + if call.args[0] == "MYPROJ-10" + ] + assert all(len(comment) <= 32767 for comment in epic_comments) + combined = "".join(epic_comments) + assert "forge-tasks-draft.json" not in combined + assert "Task One" in combined + assert "A" * 30000 in combined + assert "A" * 5000 in combined diff --git a/tests/unit/workflow/test_comment_classifier.py b/tests/unit/workflow/test_comment_classifier.py index 2bfcc7b77..f7a36b88b 100644 --- a/tests/unit/workflow/test_comment_classifier.py +++ b/tests/unit/workflow/test_comment_classifier.py @@ -92,3 +92,35 @@ def test_whitespace_only_comment_is_informational(self) -> None: """Whitespace-only comments should be informational.""" assert classify_comment(" ") == CommentType.INFORMATIONAL assert classify_comment("\n\t") == CommentType.INFORMATIONAL + + # Command detection tests + def test_command_remove(self) -> None: + """/forge remove command should be classified as a command.""" + assert classify_comment("/forge remove 2") == CommentType.COMMAND + assert classify_comment("/Forge remove abc") == CommentType.COMMAND + + def test_command_exclude(self) -> None: + """/forge exclude command should be classified as a command.""" + assert classify_comment("/forge exclude 3") == CommentType.COMMAND + + def test_command_add(self) -> None: + """/forge add command should be classified as a command.""" + assert classify_comment('/forge add summary="Implement API"') == CommentType.COMMAND + + def test_command_update(self) -> None: + """/forge update command should be classified as a command.""" + assert classify_comment('/forge update 1 summary="New Summary"') == CommentType.COMMAND + + def test_command_case_insensitive_prefix(self) -> None: + """/forge commands should be case-insensitive.""" + assert classify_comment("/FORGE remove 2") == CommentType.COMMAND + assert classify_comment(" /Forge exclude 3") == CommentType.COMMAND + + def test_command_skip_gate_is_ignored_by_classifier(self) -> None: + """skip-gate/unskip-gate are not classified as COMMAND by classify_comment.""" + assert classify_comment("/forge skip-gate build") == CommentType.INFORMATIONAL + assert classify_comment("/forge unskip-gate test") == CommentType.INFORMATIONAL + + def test_command_rebase_is_ignored_by_classifier(self) -> None: + """rebase is not classified as COMMAND by classify_comment.""" + assert classify_comment("/forge rebase") == CommentType.INFORMATIONAL diff --git a/tests/unit/workflow/test_direct_mode.py b/tests/unit/workflow/test_direct_mode.py new file mode 100644 index 000000000..5d80f5b8d --- /dev/null +++ b/tests/unit/workflow/test_direct_mode.py @@ -0,0 +1,94 @@ +"""Tests for forge:direct-mode direct ticket creation mode.""" + +from unittest.mock import MagicMock + +import pytest + +from forge.models.workflow import ForgeLabel +from forge.workflow.feature.state import create_initial_feature_state + + +class TestDirectModeDefaultsToFalse: + def test_feature_state_direct_mode_defaults_false(self): + state = create_initial_feature_state("TEST-1") + assert state.get("direct_mode") is False + + def test_feature_state_direct_mode_can_be_set_true(self): + state = create_initial_feature_state("TEST-1", direct_mode=True) + assert state["direct_mode"] is True + + +class TestBuildInitialStateDirectMode: + """Tests for direct_mode initialization from Jira payload.""" + + def _make_worker(self): + from forge.orchestrator.worker import OrchestratorWorker + + worker = OrchestratorWorker.__new__(OrchestratorWorker) + worker.settings = MagicMock() + worker.router = MagicMock() + return worker + + def _make_message(self, labels: list): + from forge.models.events import EventSource + + msg = MagicMock() + msg.ticket_key = "TEST-1" + msg.source = EventSource.JIRA + msg.event_type = "jira:issue_updated" + msg.event_id = "evt-1" + msg.retry_count = 0 + msg.payload = { + "issue": { + "fields": { + "issuetype": {"name": "Feature"}, + "labels": labels, + } + } + } + return msg + + def test_direct_mode_true_when_label_present(self): + worker = self._make_worker() + msg = self._make_message(["forge:managed", ForgeLabel.DIRECT_MODE.value]) + state = worker._build_initial_state(msg) + assert state["direct_mode"] is True + + def test_direct_mode_false_when_label_absent(self): + worker = self._make_worker() + msg = self._make_message(["forge:managed"]) + state = worker._build_initial_state(msg) + assert state["direct_mode"] is False + + +class TestDirectModeApprovalGates: + """Each approval gate routing function pauses when direct_mode=True and yolo_mode=False.""" + + def _feature_state(self, current_node: str, extra: dict = None) -> dict: + if extra is None: + extra = {} + state = create_initial_feature_state("TEST-1") + state["current_node"] = current_node + state["is_paused"] = True + state["direct_mode"] = True + state["yolo_mode"] = False + state.update(extra) + return state + + @pytest.mark.asyncio + async def test_plan_route_pauses_in_direct_mode(self): + from langgraph.graph import END + + from forge.workflow.gates.plan_approval import route_plan_approval + + state = self._feature_state("plan_approval_gate", {"epic_keys": ["EPIC-1"]}) + assert await route_plan_approval(state) == END + + @pytest.mark.asyncio + async def test_task_route_pauses_in_direct_mode(self): + from langgraph.graph import END + + from forge.workflow.gates.task_approval import route_task_approval + + state = self._feature_state("task_approval_gate", {"task_keys": ["TASK-1"]}) + assert await route_task_approval(state) == END diff --git a/tests/unit/workflow/test_yolo_mode.py b/tests/unit/workflow/test_yolo_mode.py index b05cc8d5b..d90f7272e 100644 --- a/tests/unit/workflow/test_yolo_mode.py +++ b/tests/unit/workflow/test_yolo_mode.py @@ -2,9 +2,10 @@ import pytest -from forge.models.workflow import ForgeLabel, TicketType -from forge.workflow.feature.state import create_initial_feature_state +from forge.models.workflow import ForgeLabel +from forge.queue.models import QueueMessage from forge.workflow.bug.state import create_initial_bug_state +from forge.workflow.feature.state import create_initial_feature_state class TestForgeLabelYolo: @@ -38,7 +39,9 @@ class TestBuildInitialStateYoloMode: def _make_worker(self): from unittest.mock import MagicMock + from forge.orchestrator.worker import OrchestratorWorker + worker = OrchestratorWorker.__new__(OrchestratorWorker) worker.settings = MagicMock() worker.router = MagicMock() @@ -46,7 +49,9 @@ def _make_worker(self): def _make_message(self, labels: list): from unittest.mock import MagicMock + from forge.models.events import EventSource + msg = MagicMock() msg.ticket_key = "TEST-1" msg.source = EventSource.JIRA @@ -83,7 +88,9 @@ def test_yolo_mode_false_when_no_labels(self): def test_yolo_mode_false_for_github_source(self): from unittest.mock import MagicMock + from forge.models.events import EventSource + msg = MagicMock() msg.ticket_key = "TEST-1" msg.source = EventSource.SOURCE_CONTROL @@ -99,9 +106,12 @@ def test_yolo_mode_false_for_github_source(self): class TestYoloLabelAddedMidWorkflow: """When forge:yolo is added while paused at a gate, yolo_mode is set and workflow unpauses.""" - def _make_yolo_label_message(self, current_labels: str, previous_labels: str = "") -> "QueueMessage": + def _make_yolo_label_message( + self, current_labels: str, previous_labels: str = "" + ) -> "QueueMessage": from forge.models.events import EventSource from forge.queue.models import QueueMessage + return QueueMessage( message_id="1234567890-0", event_id="test-event-yolo", @@ -139,6 +149,7 @@ def _make_gate_state(self, current_node: str, **extra) -> dict: @pytest.mark.asyncio async def test_yolo_label_addition_at_prd_gate_activates_yolo(self): from forge.orchestrator.worker import OrchestratorWorker + worker = OrchestratorWorker(consumer_name="test-worker") message = self._make_yolo_label_message( current_labels="forge:managed forge:yolo", @@ -152,6 +163,7 @@ async def test_yolo_label_addition_at_prd_gate_activates_yolo(self): @pytest.mark.asyncio async def test_yolo_label_addition_outside_gate_does_not_activate(self): from forge.orchestrator.worker import OrchestratorWorker + worker = OrchestratorWorker(consumer_name="test-worker") message = self._make_yolo_label_message( current_labels="forge:managed forge:yolo", @@ -166,6 +178,7 @@ async def test_yolo_label_addition_outside_gate_does_not_activate(self): @pytest.mark.asyncio async def test_yolo_label_already_present_does_not_re_trigger(self): from forge.orchestrator.worker import OrchestratorWorker + worker = OrchestratorWorker(consumer_name="test-worker") # forge:yolo was already in fromString — not a new addition message = self._make_yolo_label_message( @@ -184,6 +197,7 @@ class TestYoloGateRouting: def _feature_state(self, current_node: str, **extra) -> dict: from forge.workflow.feature.state import create_initial_feature_state + state = create_initial_feature_state("TEST-1") state["current_node"] = current_node state["is_paused"] = True @@ -193,28 +207,36 @@ def _feature_state(self, current_node: str, **extra) -> dict: def test_prd_route_auto_approves_in_yolo_mode(self): from forge.workflow.gates.prd_approval import route_prd_approval + state = self._feature_state("prd_approval_gate", prd_content="# PRD") assert route_prd_approval(state) == "generate_spec" def test_spec_route_auto_approves_in_yolo_mode(self): from forge.workflow.gates.spec_approval import route_spec_approval + state = self._feature_state("spec_approval_gate", spec_content="# Spec") assert route_spec_approval(state) == "decompose_epics" - def test_plan_route_auto_approves_in_yolo_mode(self): + @pytest.mark.asyncio + async def test_plan_route_auto_approves_in_yolo_mode(self): from forge.workflow.gates.plan_approval import route_plan_approval + state = self._feature_state("plan_approval_gate", epic_keys=["EPIC-1"]) - assert route_plan_approval(state) == "generate_tasks" + assert await route_plan_approval(state) == "provision_epics" - def test_task_route_auto_approves_in_yolo_mode(self): + @pytest.mark.asyncio + async def test_task_route_auto_approves_in_yolo_mode(self): from forge.workflow.gates.task_approval import route_task_approval + state = self._feature_state("task_approval_gate", task_keys=["TASK-1"]) - assert route_task_approval(state) == "task_router" + assert await route_task_approval(state) == "provision_tasks" def test_yolo_false_still_pauses_at_prd_gate(self): from langgraph.graph import END - from forge.workflow.gates.prd_approval import route_prd_approval + from forge.workflow.feature.state import create_initial_feature_state + from forge.workflow.gates.prd_approval import route_prd_approval + state = create_initial_feature_state("TEST-1") state["current_node"] = "prd_approval_gate" state["is_paused"] = True @@ -224,6 +246,7 @@ def test_yolo_false_still_pauses_at_prd_gate(self): def test_yolo_does_not_override_question_routing(self): from forge.workflow.gates.prd_approval import route_prd_approval + state = self._feature_state("prd_approval_gate", prd_content="# PRD") state["is_question"] = True state["feedback_comment"] = "?Why REST?" @@ -259,6 +282,7 @@ def _rca_state(self, **extra) -> dict: @pytest.mark.asyncio async def test_yolo_selects_option_1_without_pausing(self): from unittest.mock import AsyncMock, patch + from forge.workflow.nodes.rca_option_gate import rca_option_gate state = self._rca_state() @@ -278,6 +302,7 @@ async def test_yolo_selects_option_1_without_pausing(self): async def test_yolo_still_posts_rca_comment(self): """RCA comment is posted even in yolo mode (audit trail preserved).""" from unittest.mock import AsyncMock, patch + from forge.workflow.nodes.rca_option_gate import rca_option_gate state = self._rca_state() @@ -295,6 +320,7 @@ async def test_yolo_still_posts_rca_comment(self): async def test_non_yolo_still_pauses(self): """With yolo_mode=False, gate pauses normally.""" from unittest.mock import AsyncMock, patch + from forge.workflow.nodes.rca_option_gate import rca_option_gate state = self._rca_state(yolo_mode=False) diff --git a/tests/unit/workflow/utils/test_draft_manager.py b/tests/unit/workflow/utils/test_draft_manager.py new file mode 100644 index 000000000..73e85f56e --- /dev/null +++ b/tests/unit/workflow/utils/test_draft_manager.py @@ -0,0 +1,341 @@ +# mypy: disallow-untyped-decorators=False +"""Tests for DraftManager utility class.""" + +from datetime import UTC, datetime +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from forge.integrations.jira import JiraClient +from forge.models.draft import DraftItem, ForgeDecompositionDraft +from forge.workflow.utils.draft_manager import ( + FORGE_EPICS_DRAFT_FILENAME, + FORGE_TASKS_DRAFT_FILENAME, + DraftManager, +) + + +@pytest.fixture( + params=[ + ("epics", FORGE_EPICS_DRAFT_FILENAME), + ("tasks", FORGE_TASKS_DRAFT_FILENAME), + ] +) +def draft_config(request: pytest.FixtureRequest) -> tuple[str, str]: + """Return a tuple of (phase, filename) representing draft configurations.""" + val: tuple[str, str] = request.param + return val + + +@pytest.fixture +def sample_draft(draft_config: tuple[str, str]) -> ForgeDecompositionDraft: + """Return a valid ForgeDecompositionDraft instance matching the draft configuration.""" + phase, _ = draft_config + now = datetime.now(UTC) + return ForgeDecompositionDraft( + parent_key="PROJ-123", + phase=phase, + items=[ + DraftItem( + id=1, + summary=f"{phase.capitalize()} 1", + description="Desc 1", + repo="repo-a", + acceptance_criteria=["AC 1"], + ) + ], + version=1, + created_at=now, + updated_at=now, + ) + + +class TestDraftManager: + """Test cases for DraftManager CRUD operations on Jira parent tickets.""" + + @pytest.mark.asyncio + async def test_delete_draft_attachment_success(self, draft_config: tuple[str, str]) -> None: + """Should delete all matching attachments if found.""" + _, filename = draft_config + mock_jira = MagicMock(spec=JiraClient) + mock_jira.delete_attachments_by_name = AsyncMock(return_value=2) + + await DraftManager.delete_draft_attachment(mock_jira, "PROJ-123", filename) + + mock_jira.delete_attachments_by_name.assert_called_once_with("PROJ-123", filename) + + @pytest.mark.asyncio + async def test_delete_draft_attachment_not_found(self, draft_config: tuple[str, str]) -> None: + """Should do nothing and succeed if no matching attachment found.""" + _, filename = draft_config + mock_jira = MagicMock(spec=JiraClient) + mock_jira.delete_attachments_by_name = AsyncMock(return_value=0) + + await DraftManager.delete_draft_attachment(mock_jira, "PROJ-123", filename) + + mock_jira.delete_attachments_by_name.assert_called_once_with("PROJ-123", filename) + + @pytest.mark.asyncio + async def test_delete_draft_attachment_failure(self, draft_config: tuple[str, str]) -> None: + """Should propagate delete_attachments_by_name exception.""" + _, filename = draft_config + mock_jira = MagicMock(spec=JiraClient) + mock_jira.delete_attachments_by_name = AsyncMock(side_effect=Exception("Delete Error")) + + with pytest.raises(Exception, match="Delete Error"): + await DraftManager.delete_draft_attachment(mock_jira, "PROJ-123", filename) + + def test_format_review_comment_handles_pipes_without_table_cells(self) -> None: + """Pipe characters should not corrupt the review summary structure.""" + now = datetime.now(UTC) + draft = ForgeDecompositionDraft( + parent_key="PROJ-123", + phase="tasks", + items=[ + DraftItem( + id=1, + summary="Task with | pipe in summary", + description="Desc 1", + repo="repo|with|pipe", + acceptance_criteria=["AC 1"], + ) + ], + version=1, + created_at=now, + updated_at=now, + ) + + comment = DraftManager.format_review_comment(draft) + + assert "- **1.** Task with | pipe in summary — Repo: `repo|with|pipe`" in comment + assert "| ID | Summary |" not in comment + assert "#### 1. Task with | pipe in summary (Repo: repo|with|pipe)" in comment + + def test_review_comment_description_renders_as_readable_adf(self) -> None: + """Generated review comments retain multiline descriptions in Jira.""" + now = datetime.now(UTC) + draft = ForgeDecompositionDraft( + parent_key="PROJ-123", + phase="epics", + items=[ + DraftItem( + id=1, + summary="Readable plan", + description="Inspect the workflow\nImplement the fix\nAdd regression tests", + repo="org/repo", + acceptance_criteria=[], + ) + ], + created_at=now, + updated_at=now, + ) + + adf = JiraClient._text_to_adf(DraftManager.format_review_comment(draft)) + rendered_paragraphs = [ + "".join(part.get("text", "") for part in node.get("content", [])) + for node in adf["content"] + if node["type"] == "paragraph" + ] + + assert "Plan:" in rendered_paragraphs + assert "Inspect the workflow" in rendered_paragraphs + assert "Implement the fix" in rendered_paragraphs + assert "Add regression tests" in rendered_paragraphs + + def test_format_review_comment_visual_indicator_excluded(self) -> None: + """Should apply strikethrough formatting and *(excluded)* text for excluded items.""" + now = datetime.now(UTC) + draft = ForgeDecompositionDraft( + parent_key="PROJ-123", + phase="tasks", + items=[ + DraftItem( + id=1, + summary="Active task", + description="Desc 1", + repo="repo1", + acceptance_criteria=["AC 1"], + excluded=False, + ), + DraftItem( + id=2, + summary="Excluded task", + description="Desc 2", + repo="repo2", + acceptance_criteria=["AC 2"], + excluded=True, + ), + ], + version=1, + created_at=now, + updated_at=now, + ) + + comment = DraftManager.format_review_comment(draft) + + # Verify normal item is formatted normally + assert "- **1.** Active task — Repo: `repo1`" in comment + assert "#### 1. Active task (Repo: repo1)" in comment + + # Verify excluded item formatting in summary list + assert "- **2.** ~~Excluded task~~ *(excluded)* — Repo: `~~repo2~~`" in comment + # Verify excluded item heading summary in detail blocks + assert "#### 2. ~~Excluded task~~ *(excluded)* (Repo: repo2)" in comment + + def test_format_review_comment_condensed_exceeds_limit(self) -> None: + """Should truncate summary items and use a warning when exceeding the limit.""" + now = datetime.now(UTC) + draft = ForgeDecompositionDraft( + parent_key="PROJ-123", + phase="tasks", + items=[ + DraftItem( + id=i, + summary=f"Task {i}", + repo=f"repo{i}", + description=f"Desc {i}", + acceptance_criteria=[f"AC {i}"], + ) + for i in range(1, 11) + ], + version=1, + created_at=now, + updated_at=now, + ) + + # Set a limit that is large enough to contain headers + footer + some rows + warning note, + # but too small to fit all 10 rows. + comment = DraftManager.format_review_comment(draft, limit=850) + + assert len(comment) <= 850 + assert "⚠️ Showing first" in comment + assert "items in this comment." in comment + assert "forge-tasks-draft.json" not in comment + assert "Task 1" in comment + assert "## 🤖 Forge interaction options" in comment + + # Verify it is truncated when the limit is extremely small (e.g. 100) + very_small_comment = DraftManager.format_review_comment(draft, limit=100) + assert len(very_small_comment) <= 100 + assert very_small_comment.endswith(" [truncated]") + + def test_chunk_text_by_limit(self) -> None: + """Verify chunk_text_by_limit splits text correctly.""" + text = "Line 1\nLine 2\nLine 3" + # Split with small limit + chunks = DraftManager.chunk_text_by_limit(text, limit=10) + assert len(chunks) == 3 + assert chunks[0] == "Line 1" + assert chunks[1] == "Line 2" + assert chunks[2] == "Line 3" + + @pytest.mark.asyncio + async def test_post_task_draft_review(self) -> None: + """Verify post_task_draft_review slices tasks by Epic and posts comments to Epic and Feature tickets.""" + now = datetime.now(UTC) + draft = ForgeDecompositionDraft( + parent_key="FEATURE-1", + phase="tasks", + items=[ + DraftItem( + id=1, + summary="Task 1", + description="Desc 1", + repo="repo1", + acceptance_criteria=[], + epic_key="EPIC-101", + ), + DraftItem( + id=2, + summary="Task 2", + description="Desc 2", + repo="repo2", + acceptance_criteria=[], + epic_key="EPIC-102", + ), + ], + version=1, + created_at=now, + updated_at=now, + ) + + mock_jira = MagicMock(spec=JiraClient) + mock_jira.add_comment = AsyncMock() + + await DraftManager.post_task_draft_review(mock_jira, "FEATURE-1", draft) + + # Should add comments to EPIC-101, EPIC-102, and FEATURE-1 + assert mock_jira.add_comment.call_count == 3 + + # Verify Epic EPIC-101 comment + epic_101_call = [ + call for call in mock_jira.add_comment.call_args_list if call[0][0] == "EPIC-101" + ][0] + assert "### 📋 Proposed Tasks Draft" in epic_101_call[0][1] + assert "Task 1" in epic_101_call[0][1] + + # Verify Epic EPIC-102 comment + epic_102_call = [ + call for call in mock_jira.add_comment.call_args_list if call[0][0] == "EPIC-102" + ][0] + assert "### 📋 Proposed Tasks Draft" in epic_102_call[0][1] + assert "Task 2" in epic_102_call[0][1] + + # Verify Feature FEATURE-1 comment + feature_call = [ + call for call in mock_jira.add_comment.call_args_list if call[0][0] == "FEATURE-1" + ][0] + assert "### 📋 Proposed Tasks Drafts by Epic" in feature_call[0][1] + assert "EPIC-101" in feature_call[0][1] + assert "EPIC-102" in feature_call[0][1] + + @pytest.mark.asyncio + async def test_get_draft_attachment_success(self) -> None: + """Verify get_draft_attachment retrieves and parses the correct attachment successfully.""" + mock_jira = MagicMock(spec=JiraClient) + mock_jira.get_attachments = AsyncMock( + return_value=[ + {"id": "att-1", "filename": "other.json", "content_url": "http://other"}, + { + "id": "att-2", + "filename": "forge-epics-draft.json", + "content_url": "http://epics", + }, + ] + ) + now = datetime.now(UTC) + draft = ForgeDecompositionDraft( + parent_key="FEATURE-1", + phase="epics", + items=[], + created_at=now, + updated_at=now, + ) + mock_jira.download_attachment = AsyncMock( + return_value=draft.model_dump_json().encode("utf-8") + ) + + res = await DraftManager.get_draft_attachment( + mock_jira, "FEATURE-1", "forge-epics-draft.json" + ) + assert res is not None + assert res.parent_key == "FEATURE-1" + assert res.phase == "epics" + mock_jira.get_attachments.assert_called_once_with("FEATURE-1") + mock_jira.download_attachment.assert_called_once_with("http://epics") + + @pytest.mark.asyncio + async def test_get_draft_attachment_not_found(self) -> None: + """Verify get_draft_attachment returns None if the target attachment doesn't exist.""" + mock_jira = MagicMock(spec=JiraClient) + mock_jira.get_attachments = AsyncMock( + return_value=[ + {"id": "att-1", "filename": "other.json", "content_url": "http://other"}, + ] + ) + + res = await DraftManager.get_draft_attachment( + mock_jira, "FEATURE-1", "forge-epics-draft.json" + ) + assert res is None + mock_jira.get_attachments.assert_called_once_with("FEATURE-1") diff --git a/tests/workflow/test_draft_review_flow.py b/tests/workflow/test_draft_review_flow.py new file mode 100644 index 000000000..f1238fe65 --- /dev/null +++ b/tests/workflow/test_draft_review_flow.py @@ -0,0 +1,855 @@ +"""Integration tests for Draft Review Flow. + +Covers YOLO bypass path, draft attachment creation/cleanup, BR-003 truncation +rules, excluded item skipping during ticket provisioning, and draft retention +on partial ticket provisioning failure. +""" + +from datetime import UTC, datetime +from typing import Any +from unittest.mock import AsyncMock, patch + +import pytest + +from forge.config import Settings +from forge.models.draft import DraftItem, ForgeDecompositionDraft +from forge.workflow.gates.plan_approval import provision_epics_from_draft +from forge.workflow.gates.task_approval import provision_tasks_from_draft +from forge.workflow.nodes.epic_decomposition import decompose_epics +from forge.workflow.nodes.task_generation import generate_tasks +from forge.workflow.utils.draft_manager import DraftManager + + +@pytest.fixture +def mock_settings() -> Settings: + """Create settings for tests.""" + return Settings( + redis_url="redis://localhost:6379/0", + jira_base_url="https://test.atlassian.net", + jira_api_token="test-token", + jira_user_email="test@example.com", + jira_webhook_secret="test-webhook-secret", + github_token="test-github-token", + github_webhook_secret="test-github-webhook-secret", + llm_backend="anthropic", + llm_model="claude-sonnet-4-5-20250929", + anthropic_api_key="test-anthropic-key", + yolo_mode=False, + ) + + +@pytest.fixture +def base_epic_state() -> dict[str, Any]: + """Base state for epic decomposition.""" + return { + "ticket_key": "TEST-100", + "spec_content": "Build feature x.", + "qa_history": [], + "retry_count": 0, + "yolo_mode": False, + "epic_keys": [], + } + + +@pytest.fixture +def base_task_state() -> dict[str, Any]: + """Base state for task generation.""" + return { + "ticket_key": "TEST-100", + "spec_content": "Build feature x.", + "qa_history": [], + "retry_count": 0, + "yolo_mode": False, + "epic_keys": ["TEST-101"], + "task_keys": [], + "tasks_by_repo": {}, + } + + +@pytest.fixture +def mock_parent_issue() -> Any: + """Mock Jira parent issue.""" + from forge.integrations.jira.models import JiraIssue + + return JiraIssue( + key="TEST-123", + id="10123", + summary="Test Feature Summary", + description="Test Feature Description", + status="In Progress", + issue_type="Feature", + ) + + +@pytest.fixture +def mock_epic_issue() -> Any: + """Mock Jira Epic issue.""" + from forge.integrations.jira.models import JiraIssue + + return JiraIssue( + key="EPIC-456", + id="10456", + summary="Test Epic Summary", + description="Test Epic Plan Description", + status="In Progress", + issue_type="Epic", + parent_key="TEST-123", + ) + + +@pytest.fixture +def mock_epics_data() -> list[dict[str, Any]]: + """Mock generated epics data from LLM agent.""" + return [ + {"summary": "Epic 1", "plan": "Plan for epic 1", "repo": "acme/repo1"}, + {"summary": "Epic 2", "plan": "Plan for epic 2", "repo": "acme/repo2"}, + ] + + +@pytest.fixture +def mock_tasks_data() -> list[dict[str, Any]]: + """Mock generated tasks data from LLM agent.""" + return [ + {"summary": "Task 1", "description": "Desc for task 1", "repo": "acme/repo1"}, + {"summary": "Task 2", "description": "Desc for task 2", "repo": "acme/repo2"}, + ] + + +class TestYoloBypassPath: + """Acceptance Criterion: Integration tests verify the YOLO bypass path.""" + + @pytest.mark.asyncio + async def test_epic_decomposition_yolo_bypass( + self, + base_epic_state: dict[str, Any], + mock_parent_issue: Any, + mock_epics_data: list[dict[str, Any]], + mock_settings: Settings, + ) -> None: + """Verify decompose_epics provisions immediately and does NOT save draft attachments when YOLO is active.""" + state = {**base_epic_state, "yolo_mode": True} + + with ( + patch( + "forge.workflow.nodes.epic_decomposition.get_settings", return_value=mock_settings + ), + patch("forge.workflow.nodes.epic_decomposition.JiraClient") as MockJira, + patch("forge.workflow.nodes.epic_decomposition.ForgeAgent") as MockAgent, + patch( + "forge.workflow.nodes.epic_decomposition.DraftManager", wraps=DraftManager + ) as MockDraftManager, + patch("forge.workflow.nodes.epic_decomposition.post_qa_summary_if_needed"), + patch("forge.workflow.nodes.epic_decomposition.post_status_comment"), + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + mock_jira.get_issue = AsyncMock(return_value=mock_parent_issue) + mock_jira.get_labels = AsyncMock(return_value=["forge:managed"]) + mock_jira.get_project_repos = AsyncMock(return_value=["acme/repo1", "acme/repo2"]) + mock_jira.create_epic = AsyncMock(side_effect=["TEST-101", "TEST-102"]) + mock_jira.set_workflow_label = AsyncMock() + mock_jira.add_comment = AsyncMock() + + mock_agent = AsyncMock() + MockAgent.return_value = mock_agent + mock_agent.generate_epics = AsyncMock(return_value=mock_epics_data) + + MockDraftManager.save_draft_attachment = AsyncMock() + MockDraftManager.delete_draft_attachment = AsyncMock() + + result = await decompose_epics(state) + + # Verify immediate provisioning of Epics + assert mock_jira.create_epic.call_count == 2 + mock_jira.create_epic.assert_any_call( + project_key="TEST", + summary="Epic 1", + description="Plan for epic 1", + parent_key="TEST-100", + labels=["forge:managed", "forge:parent:TEST-100", "repo:acme/repo1"], + ) + + # Verify draft was NOT saved or cleaned up + MockDraftManager.save_draft_attachment.assert_not_called() + MockDraftManager.delete_draft_attachment.assert_not_called() + + # Verify workflow pauses state is not set, instead keys are returned and transitions + assert result["epic_keys"] == ["TEST-101", "TEST-102"] + assert result.get("is_paused") is not True + assert result["current_node"] == "plan_approval_gate" + + @pytest.mark.asyncio + async def test_task_generation_yolo_bypass( + self, + base_task_state: dict[str, Any], + mock_parent_issue: Any, + mock_epic_issue: Any, + mock_tasks_data: list[dict[str, Any]], + mock_settings: Settings, + ) -> None: + """Verify generate_tasks provisions immediately and does NOT save draft attachments when YOLO is active.""" + state = {**base_task_state, "yolo_mode": True} + + with ( + patch("forge.workflow.nodes.task_generation.get_settings", return_value=mock_settings), + patch("forge.workflow.nodes.task_generation.JiraClient") as MockJira, + patch("forge.workflow.nodes.task_generation.ForgeAgent") as MockAgent, + patch( + "forge.workflow.nodes.task_generation.DraftManager", wraps=DraftManager + ) as MockDraftManager, + patch("forge.workflow.nodes.task_generation.post_status_comment"), + patch( + "forge.workflow.nodes.task_generation._generate_tasks_for_epic", + new_callable=AsyncMock, + return_value=mock_tasks_data, + ), + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + mock_jira.get_issue = AsyncMock(side_effect=[mock_parent_issue, mock_epic_issue]) + mock_jira.get_labels = AsyncMock(return_value=["forge:managed"]) + mock_jira.create_task = AsyncMock(side_effect=["TEST-110", "TEST-111"]) + mock_jira.set_workflow_label = AsyncMock() + mock_jira.add_comment = AsyncMock() + + mock_agent = AsyncMock() + MockAgent.return_value = mock_agent + + MockDraftManager.save_draft_attachment = AsyncMock() + MockDraftManager.delete_draft_attachment = AsyncMock() + + result = await generate_tasks(state) + + # Verify immediate provisioning of Tasks + assert mock_jira.create_task.call_count == 2 + mock_jira.create_task.assert_any_call( + project_key="TEST", + summary="Task 1", + description="Desc for task 1", + parent_key="TEST-101", + labels=["forge:managed", "forge:parent:TEST-100", "repo:acme/repo1"], + ) + + # Verify draft was NOT saved or cleaned up + MockDraftManager.save_draft_attachment.assert_not_called() + MockDraftManager.delete_draft_attachment.assert_not_called() + + # Verify result state + assert result["task_keys"] == ["TEST-110", "TEST-111"] + assert result["tasks_by_repo"] == {"acme/repo1": ["TEST-110"], "acme/repo2": ["TEST-111"]} + assert result.get("is_paused") is not True + assert result["current_node"] == "task_approval_gate" + + +class TestDraftAttachmentCreationAndCleanup: + """Acceptance Criterion: Integration tests verify draft attachment creation and cleanup.""" + + @pytest.mark.asyncio + async def test_epic_decomposition_draft_review_flow( + self, + base_epic_state: dict[str, Any], + mock_parent_issue: Any, + mock_epics_data: list[dict[str, Any]], + mock_settings: Settings, + ) -> None: + """Verify that in non-YOLO mode, decompose_epics cleans up old drafts, saves the new draft JSON, posts comments, and pauses.""" + state = {**base_epic_state, "yolo_mode": False} + + with ( + patch( + "forge.workflow.nodes.epic_decomposition.get_settings", return_value=mock_settings + ), + patch("forge.workflow.nodes.epic_decomposition.JiraClient") as MockJira, + patch("forge.workflow.nodes.epic_decomposition.ForgeAgent") as MockAgent, + patch( + "forge.workflow.nodes.epic_decomposition.DraftManager", wraps=DraftManager + ) as MockDraftManager, + patch("forge.workflow.nodes.epic_decomposition.post_qa_summary_if_needed"), + patch("forge.workflow.nodes.epic_decomposition.post_status_comment"), + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + mock_jira.get_issue = AsyncMock(return_value=mock_parent_issue) + mock_jira.get_labels = AsyncMock(return_value=["forge:managed"]) + mock_jira.get_project_repos = AsyncMock(return_value=["acme/repo1", "acme/repo2"]) + mock_jira.set_workflow_label = AsyncMock() + mock_jira.add_comment = AsyncMock() + + mock_agent = AsyncMock() + MockAgent.return_value = mock_agent + mock_agent.generate_epics = AsyncMock(return_value=mock_epics_data) + + MockDraftManager.delete_draft_attachment = AsyncMock() + MockDraftManager.save_draft_attachment = AsyncMock() + + result = await decompose_epics(state) + + # 1. Verify cleanup of any old drafts + MockDraftManager.delete_draft_attachment.assert_called_once_with( + mock_jira, "TEST-100", "forge-epics-draft.json" + ) + + # 2. Verify draft attachment saving + MockDraftManager.save_draft_attachment.assert_called_once() + saved_draft = MockDraftManager.save_draft_attachment.call_args[0][2] + assert isinstance(saved_draft, ForgeDecompositionDraft) + assert saved_draft.parent_key == "TEST-100" + assert saved_draft.phase == "epics" + assert len(saved_draft.items) == 2 + assert saved_draft.items[0].summary == "Epic 1" + assert saved_draft.items[0].repo == "acme/repo1" + + # 3. Verify comments posted + assert mock_jira.add_comment.call_count == 1 + comment_body = mock_jira.add_comment.call_args[0][1] + assert "### 📋 Proposed Epics Draft" in comment_body + assert "Epic 1" in comment_body + assert "forge:plan-approved" in comment_body + + # 4. Verify workflow state transitions and pauses + assert result["is_paused"] is True + assert result["current_node"] == "plan_approval_gate" + assert result["epic_keys"] == [] + + @pytest.mark.asyncio + async def test_task_generation_draft_review_flow( + self, + base_task_state: dict[str, Any], + mock_parent_issue: Any, + mock_epic_issue: Any, + mock_tasks_data: list[dict[str, Any]], + mock_settings: Settings, + ) -> None: + """Verify task drafts stay in state while review comments are posted.""" + state = {**base_task_state, "yolo_mode": False} + + with ( + patch("forge.workflow.nodes.task_generation.get_settings", return_value=mock_settings), + patch("forge.workflow.nodes.task_generation.JiraClient") as MockJira, + patch("forge.workflow.nodes.task_generation.ForgeAgent") as MockAgent, + patch( + "forge.workflow.nodes.task_generation.DraftManager", wraps=DraftManager + ) as MockDraftManager, + patch("forge.workflow.nodes.task_generation.post_status_comment"), + patch( + "forge.workflow.nodes.task_generation._generate_tasks_for_epic", + new_callable=AsyncMock, + return_value=mock_tasks_data, + ), + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + mock_jira.get_issue = AsyncMock(side_effect=[mock_parent_issue, mock_epic_issue]) + mock_jira.get_labels = AsyncMock(return_value=["forge:managed"]) + mock_jira.set_workflow_label = AsyncMock() + mock_jira.add_comment = AsyncMock() + + mock_agent = AsyncMock() + MockAgent.return_value = mock_agent + + MockDraftManager.delete_draft_attachment = AsyncMock() + MockDraftManager.save_task_draft_with_slices = AsyncMock() + + result = await generate_tasks(state) + + # Task JSON is not uploaded or cleaned up as a Jira attachment. + MockDraftManager.delete_draft_attachment.assert_not_called() + MockDraftManager.save_task_draft_with_slices.assert_not_called() + + saved_draft = result["tasks_draft"] + assert isinstance(saved_draft, ForgeDecompositionDraft) + assert saved_draft.parent_key == "TEST-100" + assert saved_draft.phase == "tasks" + assert len(saved_draft.items) == 2 + assert saved_draft.items[0].summary == "Task 1" + assert saved_draft.items[0].repo == "acme/repo1" + + # Verify comments posted + assert mock_jira.add_comment.call_count == 2 + epic_comments = [ + args[0][1] + for args in mock_jira.add_comment.call_args_list + if "### 📋 Proposed Tasks Draft" in args[0][1] + and "Condensed" not in args[0][1] + and "by Epic" not in args[0][1] + ] + assert len(epic_comments) == 1 + comment_body = epic_comments[0] + assert "Task 1" in comment_body + assert "forge:task-approved" in comment_body + + # 4. Verify workflow state transitions and pauses + assert result["is_paused"] is True + assert result["current_node"] == "task_approval_gate" + assert result["task_keys"] == [] + + +class TestTruncationFallbackBoundaries: + """Acceptance Criterion: Integration tests verify character length and item count truncation fallback boundaries.""" + + @pytest.mark.asyncio + async def test_item_count_truncation_boundary_epics( + self, base_epic_state: dict[str, Any], mock_parent_issue: Any, mock_settings: Settings + ) -> None: + """BR-003: Verify that when item count > 15, the review comment is formatted in the condensed table format.""" + state = {**base_epic_state, "yolo_mode": False} + + # Generate 16 epics + many_epics = [ + {"summary": f"Epic {i}", "plan": f"Plan {i}", "repo": f"acme/repo{i}"} + for i in range(1, 17) + ] + + with ( + patch( + "forge.workflow.nodes.epic_decomposition.get_settings", return_value=mock_settings + ), + patch("forge.workflow.nodes.epic_decomposition.JiraClient") as MockJira, + patch("forge.workflow.nodes.epic_decomposition.ForgeAgent") as MockAgent, + patch( + "forge.workflow.nodes.epic_decomposition.DraftManager", wraps=DraftManager + ) as MockDraftManager, + patch("forge.workflow.nodes.epic_decomposition.post_qa_summary_if_needed"), + patch("forge.workflow.nodes.epic_decomposition.post_status_comment"), + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + mock_jira.get_issue = AsyncMock(return_value=mock_parent_issue) + mock_jira.get_labels = AsyncMock(return_value=["forge:managed"]) + mock_jira.get_project_repos = AsyncMock(return_value=["acme/repo1"]) + mock_jira.set_workflow_label = AsyncMock() + mock_jira.add_comment = AsyncMock() + + mock_agent = AsyncMock() + MockAgent.return_value = mock_agent + mock_agent.generate_epics = AsyncMock(return_value=many_epics) + + MockDraftManager.delete_draft_attachment = AsyncMock() + MockDraftManager.save_draft_attachment = AsyncMock() + + await decompose_epics(state) + + # Verify comment is condensed + comment_body = mock_jira.add_comment.call_args[0][1] + assert "### 📋 Proposed Epics Draft (Condensed)" in comment_body + assert "Warning" in comment_body + assert "exceeds character or size limits" in comment_body + assert "forge-epics-draft.json" in comment_body + # Detailed descriptions of items should not be present + assert "#### 1. Epic 1" not in comment_body + + @pytest.mark.asyncio + async def test_item_count_truncation_boundary_tasks( + self, + base_task_state: dict[str, Any], + mock_parent_issue: Any, + mock_epic_issue: Any, + mock_settings: Settings, + ) -> None: + """BR-003: Verify that when item count > 15, the review comment is formatted in the condensed table format for tasks.""" + state = {**base_task_state, "yolo_mode": False} + + # Generate 16 tasks + many_tasks = [ + {"summary": f"Task {i}", "description": f"Desc {i}", "repo": f"acme/repo{i}"} + for i in range(1, 17) + ] + + with ( + patch("forge.workflow.nodes.task_generation.get_settings", return_value=mock_settings), + patch("forge.workflow.nodes.task_generation.JiraClient") as MockJira, + patch("forge.workflow.nodes.task_generation.ForgeAgent") as MockAgent, + patch( + "forge.workflow.nodes.task_generation.DraftManager", wraps=DraftManager + ) as MockDraftManager, + patch("forge.workflow.nodes.task_generation.post_status_comment"), + patch( + "forge.workflow.nodes.task_generation._generate_tasks_for_epic", + new_callable=AsyncMock, + return_value=many_tasks, + ), + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + mock_jira.get_issue = AsyncMock(side_effect=[mock_parent_issue, mock_epic_issue]) + mock_jira.get_labels = AsyncMock(return_value=["forge:managed"]) + mock_jira.set_workflow_label = AsyncMock() + mock_jira.add_comment = AsyncMock() + + mock_agent = AsyncMock() + MockAgent.return_value = mock_agent + + MockDraftManager.delete_draft_attachment = AsyncMock() + MockDraftManager.save_draft_attachment = AsyncMock() + + await generate_tasks(state) + + # Verify comment is condensed (find the comment posted to the Epic ticket) + epic_comments = [ + args[0][1] + for args in mock_jira.add_comment.call_args_list + if "### 📋 Proposed Tasks Draft" in args[0][1] + ] + assert len(epic_comments) > 0, "No draft comment found" + comment_body = epic_comments[0] + assert "### 📋 Proposed Tasks Draft (Condensed)" not in comment_body + assert "forge-tasks-draft.json" not in comment_body + assert "#### 1. Task 1" in comment_body + assert "#### 16. Task 16" in comment_body + + @pytest.mark.asyncio + async def test_character_length_truncation_boundary_epics( + self, base_epic_state: dict[str, Any], mock_parent_issue: Any, mock_settings: Settings + ) -> None: + """BR-003: Verify that when comment character length > 32,767 characters, it falls back to a condensed table.""" + state = {**base_epic_state, "yolo_mode": False} + + # Create 1 huge plan for an epic + long_plan = "A" * 33000 + epics_with_long_plan = [ + {"summary": "Epic 1", "plan": long_plan, "repo": "acme/repo1"}, + {"summary": "Epic 2", "plan": "Short plan", "repo": "acme/repo2"}, + ] + + with ( + patch( + "forge.workflow.nodes.epic_decomposition.get_settings", return_value=mock_settings + ), + patch("forge.workflow.nodes.epic_decomposition.JiraClient") as MockJira, + patch("forge.workflow.nodes.epic_decomposition.ForgeAgent") as MockAgent, + patch( + "forge.workflow.nodes.epic_decomposition.DraftManager", wraps=DraftManager + ) as MockDraftManager, + patch("forge.workflow.nodes.epic_decomposition.post_qa_summary_if_needed"), + patch("forge.workflow.nodes.epic_decomposition.post_status_comment"), + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + mock_jira.get_issue = AsyncMock(return_value=mock_parent_issue) + mock_jira.get_labels = AsyncMock(return_value=["forge:managed"]) + mock_jira.get_project_repos = AsyncMock(return_value=["acme/repo1", "acme/repo2"]) + mock_jira.set_workflow_label = AsyncMock() + mock_jira.add_comment = AsyncMock() + + mock_agent = AsyncMock() + MockAgent.return_value = mock_agent + mock_agent.generate_epics = AsyncMock(return_value=epics_with_long_plan) + + MockDraftManager.delete_draft_attachment = AsyncMock() + MockDraftManager.save_draft_attachment = AsyncMock() + + await decompose_epics(state) + + # Verify comment is condensed due to length limit + comment_body = mock_jira.add_comment.call_args[0][1] + assert "### 📋 Proposed Epics Draft (Condensed)" in comment_body + assert "Warning" in comment_body + assert "exceeds character or size limits" in comment_body + assert "forge-epics-draft.json" in comment_body + # Detailed descriptions of items should not be present + assert "#### 1. Epic 1" not in comment_body + + +class TestApprovalCommandAndSkippingExcludedItems: + """Acceptance Criterion: Integration tests verify that excluded: true items are skipped during provisioning.""" + + @pytest.mark.asyncio + async def test_epics_provisioning_skips_excluded_items( + self, mock_parent_issue: Any, mock_settings: Settings + ) -> None: + """Verify only non-excluded items are provisioned and the attachment is deleted upon success.""" + # Create draft where item 2 is excluded + draft = ForgeDecompositionDraft( + parent_key="TEST-100", + phase="epics", + items=[ + DraftItem( + id=1, + summary="Epic 1", + description="Plan 1", + repo="acme/repo1", + excluded=False, + acceptance_criteria=[], + ), + DraftItem( + id=2, + summary="Epic 2", + description="Plan 2", + repo="acme/repo2", + excluded=True, + acceptance_criteria=[], + ), + DraftItem( + id=3, + summary="Epic 3", + description="Plan 3", + repo="acme/repo3", + excluded=False, + acceptance_criteria=[], + ), + ], + version=1, + created_at=datetime.now(UTC), + updated_at=datetime.now(UTC), + ) + + state = { + "ticket_key": "TEST-100", + "is_paused": False, + "epic_keys": [], + "plan_draft": draft, + } + + with ( + patch("forge.config.get_settings", return_value=mock_settings), + patch("forge.integrations.jira.client.JiraClient") as MockJira, + patch("forge.workflow.utils.draft_manager.DraftManager") as MockDraftManager, + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + mock_jira.get_issue = AsyncMock(return_value=mock_parent_issue) + mock_jira.create_epic = AsyncMock(side_effect=["EPIC-1", "EPIC-3"]) + mock_jira.close = AsyncMock() + + MockDraftManager.get_draft_attachment = AsyncMock(return_value=draft) + MockDraftManager.delete_draft_attachment = AsyncMock() + + result_keys = await provision_epics_from_draft(state, mock_jira) + + # Verify only Epic 1 and Epic 3 were created + assert mock_jira.create_epic.call_count == 2 + mock_jira.create_epic.assert_any_call( + project_key="TEST", + summary="Epic 1", + description="Plan 1", + parent_key="TEST-100", + labels=["forge:managed", "forge:parent:TEST-100", "repo:acme/repo1"], + ) + mock_jira.create_epic.assert_any_call( + project_key="TEST", + summary="Epic 3", + description="Plan 3", + parent_key="TEST-100", + labels=["forge:managed", "forge:parent:TEST-100", "repo:acme/repo3"], + ) + + # Verify Epic 2 was skipped + for call in mock_jira.create_epic.call_args_list: + assert "Epic 2" not in call[1]["summary"] + + # Verify draft was deleted after successful provisioning + MockDraftManager.delete_draft_attachment.assert_called_once_with( + mock_jira, "TEST-100", "forge-epics-draft.json" + ) + assert result_keys == ["EPIC-1", "EPIC-3"] + + @pytest.mark.asyncio + async def test_tasks_provisioning_skips_excluded_items( + self, mock_parent_issue: Any, mock_settings: Settings + ) -> None: + """Verify only non-excluded tasks are provisioned and the attachment is deleted upon success.""" + # Create draft where item 2 is excluded + draft = ForgeDecompositionDraft( + parent_key="TEST-100", + phase="tasks", + items=[ + DraftItem( + id=1, + summary="Task 1", + description="Desc 1", + repo="acme/repo1", + excluded=False, + epic_key="EPIC-10", + acceptance_criteria=[], + ), + DraftItem( + id=2, + summary="Task 2", + description="Desc 2", + repo="acme/repo2", + excluded=True, + epic_key="EPIC-10", + acceptance_criteria=[], + ), + DraftItem( + id=3, + summary="Task 3", + description="Desc 3", + repo="acme/repo3", + excluded=False, + epic_key="EPIC-10", + acceptance_criteria=[], + ), + ], + version=1, + created_at=datetime.now(UTC), + updated_at=datetime.now(UTC), + ) + + state = { + "ticket_key": "TEST-100", + "is_paused": False, + "task_keys": [], + "epic_keys": ["EPIC-10"], + "tasks_draft": draft, + } + + with ( + patch("forge.config.get_settings", return_value=mock_settings), + patch("forge.integrations.jira.client.JiraClient") as MockJira, + patch("forge.workflow.utils.draft_manager.DraftManager") as MockDraftManager, + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + mock_jira.get_issue = AsyncMock(return_value=mock_parent_issue) + mock_jira.create_task = AsyncMock(side_effect=["TASK-1", "TASK-3"]) + mock_jira.close = AsyncMock() + + MockDraftManager.get_draft_attachment = AsyncMock(return_value=draft) + MockDraftManager.delete_draft_attachment = AsyncMock() + + task_keys, tasks_by_repo = await provision_tasks_from_draft(state, mock_jira) + + # Verify only Task 1 and Task 3 were created + assert mock_jira.create_task.call_count == 2 + mock_jira.create_task.assert_any_call( + project_key="TEST", + summary="Task 1", + description="Desc 1", + parent_key="EPIC-10", + labels=["forge:managed", "forge:parent:TEST-100", "repo:acme/repo1"], + ) + + # Task drafts live only in workflow state; no attachment cleanup occurs. + MockDraftManager.delete_draft_attachment.assert_not_called() + assert task_keys == ["TASK-1", "TASK-3"] + assert tasks_by_repo == {"acme/repo1": ["TASK-1"], "acme/repo3": ["TASK-3"]} + + +class TestDraftRetentionOnFailure: + """Acceptance Criterion: Integration tests verify draft retention on partial ticket provisioning failure.""" + + @pytest.mark.asyncio + async def test_epics_provisioning_retains_draft_on_failure( + self, mock_parent_issue: Any, mock_settings: Settings + ) -> None: + """Verify draft is retained (delete_draft_attachment is not called) if epic provisioning fails midway.""" + # Create draft with 2 epics + draft = ForgeDecompositionDraft( + parent_key="TEST-100", + phase="epics", + items=[ + DraftItem( + id=1, + summary="Epic 1", + description="Plan 1", + repo="acme/repo1", + excluded=False, + acceptance_criteria=[], + ), + DraftItem( + id=2, + summary="Epic 2", + description="Plan 2", + repo="acme/repo2", + excluded=False, + acceptance_criteria=[], + ), + ], + version=1, + created_at=datetime.now(UTC), + updated_at=datetime.now(UTC), + ) + + state = { + "ticket_key": "TEST-100", + "is_paused": False, + "epic_keys": [], + "plan_draft": draft, + } + + with ( + patch("forge.config.get_settings", return_value=mock_settings), + patch("forge.integrations.jira.client.JiraClient") as MockJira, + patch("forge.workflow.utils.draft_manager.DraftManager") as MockDraftManager, + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + mock_jira.get_issue = AsyncMock(return_value=mock_parent_issue) + # Epic 1 succeeds, Epic 2 fails with API error + mock_jira.create_epic = AsyncMock(side_effect=["EPIC-1", Exception("Jira API Failure")]) + mock_jira.close = AsyncMock() + + MockDraftManager.get_draft_attachment = AsyncMock(return_value=draft) + MockDraftManager.delete_draft_attachment = AsyncMock() + + with pytest.raises(Exception, match="Jira API Failure"): + from forge.workflow.gates.plan_approval import provision_epics + await provision_epics(state) + + # Verify delete_draft_attachment was NEVER called, thus retaining the draft + MockDraftManager.delete_draft_attachment.assert_not_called() + + @pytest.mark.asyncio + async def test_tasks_provisioning_retains_draft_on_failure( + self, mock_parent_issue: Any, mock_settings: Settings + ) -> None: + """Verify draft is retained (delete_draft_attachment is not called) if task provisioning fails midway.""" + # Create draft with 2 tasks + draft = ForgeDecompositionDraft( + parent_key="TEST-100", + phase="tasks", + items=[ + DraftItem( + id=1, + summary="Task 1", + description="Desc 1", + repo="acme/repo1", + excluded=False, + epic_key="EPIC-10", + acceptance_criteria=[], + ), + DraftItem( + id=2, + summary="Task 2", + description="Desc 2", + repo="acme/repo2", + excluded=False, + epic_key="EPIC-10", + acceptance_criteria=[], + ), + ], + version=1, + created_at=datetime.now(UTC), + updated_at=datetime.now(UTC), + ) + + state = { + "ticket_key": "TEST-100", + "is_paused": False, + "task_keys": [], + "epic_keys": ["EPIC-10"], + "tasks_draft": draft, + } + + with ( + patch("forge.config.get_settings", return_value=mock_settings), + patch("forge.integrations.jira.client.JiraClient") as MockJira, + patch("forge.workflow.utils.draft_manager.DraftManager") as MockDraftManager, + ): + mock_jira = AsyncMock() + MockJira.return_value = mock_jira + mock_jira.get_issue = AsyncMock(return_value=mock_parent_issue) + # Task 1 succeeds, Task 2 fails with API error + mock_jira.create_task = AsyncMock(side_effect=["TASK-1", Exception("Jira API Failure")]) + mock_jira.close = AsyncMock() + + MockDraftManager.get_draft_attachment = AsyncMock(return_value=draft) + MockDraftManager.delete_draft_attachment = AsyncMock() + + with pytest.raises(Exception, match="Jira API Failure"): + from forge.workflow.gates.task_approval import provision_tasks + await provision_tasks(state) + + # Verify delete_draft_attachment was NEVER called, thus retaining the draft + MockDraftManager.delete_draft_attachment.assert_not_called() diff --git a/tests/workflow/utils/test_comment_command.py b/tests/workflow/utils/test_comment_command.py new file mode 100644 index 000000000..6bdd44820 --- /dev/null +++ b/tests/workflow/utils/test_comment_command.py @@ -0,0 +1,467 @@ +"""Tests for parse_comment_command functionality.""" + +from typing import Any + +import pytest + +from forge.workflow.utils import parse_comment_command + + +def test_parse_remove_command_success() -> None: + """Test successful parsing of remove command.""" + result = parse_comment_command("/forge remove 2") + assert result == {"command": "remove", "id": 2} + + # Case insensitivity + result = parse_comment_command(" /FORGE Remove 42 ") + assert result == {"command": "remove", "id": 42} + + +def test_parse_remove_command_failures() -> None: + """Test parsing failures of remove command.""" + # Missing ID + result = parse_comment_command("/forge remove") + assert result is not None + assert "error" in result + assert result["command"] == "remove" + + # Invalid ID (string) + result = parse_comment_command("/forge remove abc") + assert result is not None + assert "error" in result + assert result["command"] == "remove" + + # Invalid ID (negative) + result = parse_comment_command("/forge remove -5") + assert result is not None + assert "error" in result + assert result["command"] == "remove" + + +def test_parse_exclude_command_success() -> None: + """Test successful parsing of exclude command.""" + result = parse_comment_command("/forge exclude 3") + assert result == {"command": "exclude", "id": 3} + + +def test_parse_exclude_command_failures() -> None: + """Test parsing failures of exclude command.""" + result = parse_comment_command("/forge exclude") + assert result is not None + assert "error" in result + assert result["command"] == "exclude" + + result = parse_comment_command("/forge exclude xyz") + assert result is not None + assert "error" in result + assert result["command"] == "exclude" + + +def test_parse_approve_command_removed() -> None: + """Test that approve command is no longer recognized and returns None.""" + assert parse_comment_command("/forge approve") is None + assert parse_comment_command(" /FORGE approve ") is None + assert parse_comment_command("/forge approve 1") is None + + +def test_parse_add_command_success() -> None: + """Test successful parsing of add command.""" + result = parse_comment_command( + '/forge add summary="Implement API" repo="core-api" description="Set up endpoints"' + ) + assert result == { + "command": "add", + "params": { + "summary": "Implement API", + "repo": "core-api", + "description": "Set up endpoints", + }, + } + + # Mix of double, single and no quotes + result = parse_comment_command("/forge add summary='test single' count=42 name=\"quoted\"") + assert result == { + "command": "add", + "params": { + "summary": "test single", + "count": "42", + "name": "quoted", + }, + } + + +def test_parse_add_command_failures() -> None: + """Test parsing failures of add command.""" + # Missing parameters + result = parse_comment_command("/forge add") + assert result is not None + assert "error" in result + assert result["command"] == "add" + + # Malformed parameter (no key) + result = parse_comment_command("/forge add =value") + assert result is not None + assert "error" in result + assert result["command"] == "add" + + # Malformed parameters (trailing junk) + result = parse_comment_command('/forge add key="value" junk') + assert result is not None + assert "error" in result + assert result["command"] == "add" + + +def test_parse_update_command_success() -> None: + """Test successful parsing of update command.""" + result = parse_comment_command('/forge update 1 summary="New Summary"') + assert result == { + "command": "update", + "id": 1, + "params": {"summary": "New Summary"}, + } + + result = parse_comment_command("/forge update 100") + assert result == { + "command": "update", + "id": 100, + "params": {}, + } + + +def test_parse_update_command_failures() -> None: + """Test parsing failures of update command.""" + # Missing everything + result = parse_comment_command("/forge update") + assert result is not None + assert "error" in result + assert result["command"] == "update" + + # Missing ID but has parameters + result = parse_comment_command('/forge update summary="test"') + assert result is not None + assert "error" in result + assert result["command"] == "update" + + # Invalid ID + result = parse_comment_command('/forge update abc summary="test"') + assert result is not None + assert "error" in result + assert result["command"] == "update" + + # Malformed parameters + result = parse_comment_command('/forge update 1 summary="test" junk') + assert result is not None + assert "error" in result + assert result["command"] == "update" + + +def test_parse_command_non_matching() -> None: + """Test that unrelated texts or other /forge commands return None.""" + assert parse_comment_command("/forge skip-gate build") is None + assert parse_comment_command("/forge unskip-gate test") is None + assert parse_comment_command("/forge rebase") is None + assert parse_comment_command("/forge foo") is None + assert parse_comment_command("?what is this?") is None + assert parse_comment_command("!please update") is None + assert parse_comment_command("") is None + + +@pytest.fixture +def sample_draft_json() -> list[dict[str, Any]]: + return [ + { + "id": 1, + "summary": "Implement login route", + "description": "Create a POST route for user login", + "repo": "auth-api", + "acceptance_criteria": ["POST /login returns JWT on success"], + "excluded": False, + }, + { + "id": 2, + "summary": "Implement signup route", + "description": "Create a POST route for user registration", + "repo": "auth-api", + "acceptance_criteria": ["POST /signup registers user"], + "excluded": False, + }, + { + "id": 3, + "summary": "Add database migration", + "description": "Write migration script for users table", + "repo": "db-migration", + "acceptance_criteria": ["Users table has id, email, password"], + "excluded": True, + }, + ] + + +def test_apply_draft_modification_remove_success(sample_draft_json) -> None: + """Test successful removal and re-sequencing.""" + from forge.workflow.utils.draft_manager import DraftManager + + parsed_cmd = {"command": "remove", "id": 2} + result = DraftManager.apply_draft_modification(sample_draft_json, parsed_cmd) + + assert len(result) == 2 + # Verify remaining items are re-sequenced + assert result[0]["id"] == 1 + assert result[0]["summary"] == "Implement login route" + assert result[1]["id"] == 2 + assert result[1]["summary"] == "Add database migration" + + +def test_apply_draft_modification_remove_missing_id(sample_draft_json) -> None: + """Test that removal fails if ID is missing.""" + from forge.workflow.utils.draft_manager import DraftManager + + parsed_cmd = {"command": "remove"} + with pytest.raises(ValueError, match="Missing ID for removal"): + DraftManager.apply_draft_modification(sample_draft_json, parsed_cmd) + + +def test_apply_draft_modification_remove_not_found(sample_draft_json) -> None: + """Test that removal fails if ID is not found.""" + from forge.workflow.utils.draft_manager import DraftManager + + parsed_cmd = {"command": "remove", "id": 99} + with pytest.raises(ValueError, match="Item with ID 99 not found for removal"): + DraftManager.apply_draft_modification(sample_draft_json, parsed_cmd) + + +def test_apply_draft_modification_add_success(sample_draft_json) -> None: + """Test successful addition with next sequential ID.""" + from forge.workflow.utils.draft_manager import DraftManager + + parsed_cmd = { + "command": "add", + "params": { + "summary": "New task", + "description": "Task description", + "repo": "test-repo", + "acceptance_criteria": ["Criteria 1", "Criteria 2"], + "epic_key": "EPIC-123", + }, + } + result = DraftManager.apply_draft_modification(sample_draft_json, parsed_cmd) + + assert len(result) == 4 + new_item = result[-1] + assert new_item["id"] == 4 + assert new_item["summary"] == "New task" + assert new_item["description"] == "Task description" + assert new_item["repo"] == "test-repo" + assert new_item["acceptance_criteria"] == ["Criteria 1", "Criteria 2"] + assert new_item["excluded"] is False + assert new_item["epic_key"] == "EPIC-123" + + +def test_apply_draft_modification_add_defaults(sample_draft_json) -> None: + """Test addition using only some parameters, relying on defaults for others.""" + from forge.workflow.utils.draft_manager import DraftManager + + parsed_cmd = { + "command": "add", + "params": { + "summary": "Minimal task", + }, + } + result = DraftManager.apply_draft_modification(sample_draft_json, parsed_cmd) + + assert len(result) == 4 + new_item = result[-1] + assert new_item["id"] == 4 + assert new_item["summary"] == "Minimal task" + assert new_item["description"] == "" + assert new_item["repo"] == "" + assert new_item["acceptance_criteria"] == [] + assert new_item["excluded"] is False + + +def test_apply_draft_modification_update_success(sample_draft_json) -> None: + """Test successful update of target fields.""" + from forge.workflow.utils.draft_manager import DraftManager + + parsed_cmd = { + "command": "update", + "id": 2, + "params": { + "summary": "Updated summary", + "acceptance_criteria": ["New AC"], + "excluded": True, + }, + } + result = DraftManager.apply_draft_modification(sample_draft_json, parsed_cmd) + + assert len(result) == 3 + updated_item = result[1] + assert updated_item["id"] == 2 + assert updated_item["summary"] == "Updated summary" + # Unchanged fields remain + assert updated_item["description"] == "Create a POST route for user registration" + assert updated_item["repo"] == "auth-api" + assert updated_item["acceptance_criteria"] == ["New AC"] + assert updated_item["excluded"] is True + + +def test_apply_draft_modification_update_missing_id(sample_draft_json) -> None: + """Test update raises error if ID is missing.""" + from forge.workflow.utils.draft_manager import DraftManager + + parsed_cmd = {"command": "update", "params": {"summary": "No ID"}} + with pytest.raises(ValueError, match="Missing ID for update"): + DraftManager.apply_draft_modification(sample_draft_json, parsed_cmd) + + +def test_apply_draft_modification_update_not_found(sample_draft_json) -> None: + """Test update raises error if ID is not found.""" + from forge.workflow.utils.draft_manager import DraftManager + + parsed_cmd = {"command": "update", "id": 99, "params": {"summary": "Not found"}} + with pytest.raises(ValueError, match="Item with ID 99 not found for update"): + DraftManager.apply_draft_modification(sample_draft_json, parsed_cmd) + + +def test_apply_draft_modification_exclude_success(sample_draft_json) -> None: + """Test flipping the excluded boolean key.""" + from forge.workflow.utils.draft_manager import DraftManager + + # Flip from False to True + parsed_cmd1 = {"command": "exclude", "id": 1} + result1 = DraftManager.apply_draft_modification(sample_draft_json, parsed_cmd1) + assert result1[0]["excluded"] is True + + # Flip from True to False + parsed_cmd2 = {"command": "exclude", "id": 3} + result2 = DraftManager.apply_draft_modification(sample_draft_json, parsed_cmd2) + assert result2[2]["excluded"] is False + + # Flip when excluded field is completely missing (defaults to False, so flips to True) + draft_without_excluded = [ + { + "id": 1, + "summary": "No excluded field", + "description": "Desc", + "repo": "repo", + "acceptance_criteria": [], + } + ] + parsed_cmd3 = {"command": "exclude", "id": 1} + result3 = DraftManager.apply_draft_modification(draft_without_excluded, parsed_cmd3) + assert result3[0]["excluded"] is True + + +def test_apply_draft_modification_exclude_missing_id(sample_draft_json) -> None: + """Test exclude raises error if ID is missing.""" + from forge.workflow.utils.draft_manager import DraftManager + + parsed_cmd = {"command": "exclude"} + with pytest.raises(ValueError, match="Missing ID for exclude command"): + DraftManager.apply_draft_modification(sample_draft_json, parsed_cmd) + + +def test_apply_draft_modification_exclude_not_found(sample_draft_json) -> None: + """Test exclude raises error if ID is not found.""" + from forge.workflow.utils.draft_manager import DraftManager + + parsed_cmd = {"command": "exclude", "id": 99} + with pytest.raises(ValueError, match="Item with ID 99 not found for exclude"): + DraftManager.apply_draft_modification(sample_draft_json, parsed_cmd) + + +@pytest.mark.parametrize( + "invalid_params, expected_error", + [ + ({"summary": 123}, "Field 'summary' must be a string"), + ({"description": ["not a string"]}, "Field 'description' must be a string"), + ({"repo": True}, "Field 'repo' must be a string"), + ( + {"acceptance_criteria": "string-instead-of-list"}, + "Field 'acceptance_criteria' must be a list of strings", + ), + ({"acceptance_criteria": [123]}, "Field 'acceptance_criteria' must be a list of strings"), + ({"excluded": "True"}, "Field 'excluded' must be a boolean"), + ({"unknown_field": "some-val"}, "Unknown field 'unknown_field'"), + ], +) +def test_apply_draft_modification_type_validation_add( + sample_draft_json, invalid_params, expected_error +) -> None: + """Test strict type validation for the 'add' command.""" + from forge.workflow.utils.draft_manager import DraftManager + + parsed_cmd = {"command": "add", "params": invalid_params} + with pytest.raises(ValueError, match=expected_error): + DraftManager.apply_draft_modification(sample_draft_json, parsed_cmd) + + +@pytest.mark.parametrize( + "invalid_params, expected_error", + [ + ({"summary": 123}, "Field 'summary' must be a string"), + ({"description": ["not a string"]}, "Field 'description' must be a string"), + ({"repo": True}, "Field 'repo' must be a string"), + ( + {"acceptance_criteria": "string-instead-of-list"}, + "Field 'acceptance_criteria' must be a list of strings", + ), + ({"acceptance_criteria": [123]}, "Field 'acceptance_criteria' must be a list of strings"), + ({"excluded": "True"}, "Field 'excluded' must be a boolean"), + ({"unknown_field": "some-val"}, "Unknown field 'unknown_field'"), + ], +) +def test_apply_draft_modification_type_validation_update( + sample_draft_json, invalid_params, expected_error +) -> None: + """Test strict type validation for the 'update' command.""" + from forge.workflow.utils.draft_manager import DraftManager + + parsed_cmd = {"command": "update", "id": 2, "params": invalid_params} + with pytest.raises(ValueError, match=expected_error): + DraftManager.apply_draft_modification(sample_draft_json, parsed_cmd) + + +def test_apply_draft_modification_parsing_error(sample_draft_json) -> None: + """Test that if the parsed_command dictionary contains an 'error' key, ValueError is raised.""" + from forge.workflow.utils.draft_manager import DraftManager + + parsed_cmd = { + "command": "remove", + "error": "Missing integer ID for remove command", + } + with pytest.raises( + ValueError, match="Invalid command parameters: Missing integer ID for remove command" + ): + DraftManager.apply_draft_modification(sample_draft_json, parsed_cmd) + + +def test_apply_draft_modification_deepcopy_isolation(sample_draft_json) -> None: + """Test that apply_draft_modification doesn't modify the input draft_json in place.""" + from forge.workflow.utils.draft_manager import DraftManager + + parsed_cmd = { + "command": "update", + "id": 1, + "params": { + "summary": "Completely new summary", + }, + } + import copy + + original_copy = copy.deepcopy(sample_draft_json) + + result = DraftManager.apply_draft_modification(sample_draft_json, parsed_cmd) + + assert result[0]["summary"] == "Completely new summary" + assert sample_draft_json == original_copy + + +def test_apply_draft_modification_invalid_command_failures(sample_draft_json) -> None: + """Test that invalid command types raise ValueError.""" + from forge.workflow.utils.draft_manager import DraftManager + + with pytest.raises(ValueError, match="Command type is missing in parsed command."): + DraftManager.apply_draft_modification(sample_draft_json, {}) + + with pytest.raises(ValueError, match="Unsupported modification command type: 'invalid'"): + DraftManager.apply_draft_modification(sample_draft_json, {"command": "invalid"})