[AISOS-2294] Add decompose draft review step before Jira task creation by ekuris-redhat · Pull Request #242 · forge-sdlc/forge

ekuris-redhat · 2026-07-30T16:06:00Z

Summary

This pull request implements a streamlined planning review and approval flow for epic decomposition and task generation workflows. It leverages the LangGraph FeatureState as the single atomic source of truth to manage proposed items, supplemented by derived, one-way writes of updated drafts to Jira attachments to keep backups in sync, and attaching sliced task drafts to individual Epic tickets. Transition gates are resolved through label-based approvals (forge:plan-approved and forge:task-approved) or comment-based commands (such as /forge approve), while revision requests and natural language feedback are captured via leading whitespace-tolerant comments starting with !. This simplifies the human-in-the-loop validation and regeneration loops, directly provisioning Epic and Task tickets in Jira under structured orchestration.

Changes

Workflow State Management

Utilizes LangGraph FeatureState as the single atomic source of truth for planning and review states, supplemented by derived, one-way writes of updated drafts to Jira attachments to keep backups in sync.
Renamed references from stories draft phase to epics to keep naming consistent with the domain vocabulary, while retaining support for 'stories' as a valid phase to ensure backward compatibility.
Resolved strict mypy type check warnings and redefinitions in the planning/task approval gates (e.g., casting return states, explicitly typing the set epic_keys_to_clean, and renaming duplicated variables in orchestrator worker resume blocks).

Natural Language Feedback & Comment Classification

Enhanced comment classification in src/forge/workflow/utils/comment_classifier.py to identify comments starting with a ! prefix as CommentType.FEEDBACK, comments with a ? prefix as CommentType.QUESTION, and /forge commands as CommentType.COMMAND (supporting remove, exclude, approve, add, and update commands).
Integrated both label-based approvals (forge:plan-approved, forge:task-approved) and comment-based commands (such as /forge approve) to trigger direct ticket provisioning.
Refactored command and revision comment handlers to cleanly map child Epic comments onto the Feature state while safely handling ADF/ADF-derived text.

Orchestration & State Machine Integration

Modified src/forge/workflow/nodes/epic_decomposition.py and src/forge/workflow/nodes/task_generation.py to conditionally enter the plan/task approval gates, pausing execution at PENDING_APPROVAL to wait for webhook events.
Added warning logs during task generation on task reparenting due to a missing parent epic_key.
Removed separate docs repository implementation from update_docs_repo.py and reverted type-safety changes in docs_updater.py to match the main branch state.
Removed unused checkout_commit and has_commits_ahead methods from GitOperations in src/forge/workspace/git_ops.py to clean up dead code.
Supported direct ticket creation mode without full YOLO mode via the forge:direct-mode label, which slices task drafts per Epic and attaches epic-specific forge-tasks-draft.json files to individual Epic tickets, and automatically deletes them upon provisioning.
Stopped tracking the Pipfile and .mypy_cache/ directory in Git to prevent cache pollution and index bloat, preserving pyproject.toml and uv.lock as the correct package definition files.

Documentation Updates

Updated CLAUDE.md, docs/guide/labels.md, docs/guide/feature-workflow.md, and docs/developer-guide.md to reflect comment-based commands, label-based approvals, revision comments, and updated workflow stage transitions.

Implementation Notes

Single Source of Truth (FeatureState): By utilizing the LangGraph state checkpointing mechanism, the plan status and generated epics/tasks are managed as part of the state context. For synchronicity, a derived, one-way write of updated drafts is made back to Jira attachments during interactive edits.
Label and Comment-Based Approvals: Planning gates are approved through either the use of Jira labels (forge:plan-approved, forge:task-approved) or comment-based commands (such as /forge approve).
Streamlined Docs and Git Integration: Cleans up unused checkout_commit and has_commits_ahead git methods, and simplifies the docs updater implementation by removing separate docs repository handling and inlining workspace exclude logic to maintain alignment with the main branch.
Global YOLO Bypass & Direct Mode: Detects the forge:yolo label to automatically skip the planning approval gates and provision issues immediately, while cleaning up global yolo_mode settings checks. Additionally, supports a forge:direct-mode label for direct ticket creation without full YOLO mode.

Testing

Unit Testing: Covered model validation sequences, comment classification, custom mock LLM agents, and state machine transition logic.
Integration Testing: Migrated legacy integration tests in tests/integration/orchestrator/test_workflow_execution.py to the pluggable workflows and configured proper mocks.
Test Locations:
- tests/unit/models/test_draft.py
- tests/unit/workflow/test_comment_classifier.py
- tests/unit/workflow/utils/test_draft_manager.py
- tests/workflow/utils/test_comment_command.py
- tests/unit/integrations/jira/test_client_attachments.py
- tests/unit/integrations/agents/test_agent.py
- tests/unit/orchestrator/gates/test_plan_approval.py
- tests/unit/orchestrator/gates/test_task_approval.py
- tests/unit/workflow/nodes/test_epic_decomposition.py
- tests/unit/workflow/nodes/test_task_generation.py
- tests/unit/orchestrator/test_worker.py
- tests/integration/orchestrator/test_workflow_execution.py
- tests/workflow/test_draft_review_flow.py
- tests/sandbox/test_task_execution.py

Related Tickets

Generated by Forge SDLC Orchestrator

ekuris-redhat

! Please address the following issues found during review:

Missing empty-draft guard in epic decomposition (non-YOLO path)

In src/forge/workflow/nodes/task_generation.py, the non-YOLO path correctly checks if not proposed_tasks_list: and returns a retry state
before touching Jira. The equivalent path in src/forge/workflow/nodes/epic_decomposition.py has no such check. If the agent returns
zero epics in non-YOLO mode, the code creates an empty ForgeDecompositionDraft, uploads it, posts a comment with an empty table, and
pauses the workflow with nothing for the human to approve. Please add the same guard before the draft creation block in decompose_epics:
if epics_data is empty after the LLM call, return a retry state with an appropriate last_error, matching the YOLO path's else branch.

edit_comment skips retry logic

In src/forge/integrations/jira/client.py, the edit_comment method uses client = await self._get_client() and calls client.put(...)
directly. Every other new Jira helper added in this PR (download_attachment, delete_attachment, get_attachments, add_attachment) routes
through _request_with_retry. The PR explicitly advertises rate-limit and retry logic as a feature, but edit_comment will fail
immediately on transient 429 responses. Please refactor it to use _request_with_retry like the other helpers.

Revision comment detection inconsistency

In src/forge/orchestrator/worker.py, the revision comment check uses comment_body.startswith("!"). The classify_comment function in
comment_classifier.py uses _REVISION_PATTERN = re.compile(r"^\s*!") which allows leading whitespace. A comment with a leading space
would be classified as FEEDBACK by the classifier but not caught as a revision comment by the worker, falling through to trigger full
regeneration instead of draft JSON revision. Please change the worker check to use bool(re.match(r"^\s*!", comment_body)) to match the
classifier.

ekuris-redhat · 2026-07-30T16:56:32Z

Forge is addressing PR review feedback now. This status update is informational.

ekuris-redhat · 2026-07-30T16:57:33Z

Forge is addressing PR review feedback now. This status update is informational.

ekuris-redhat · 2026-07-30T17:16:37Z

Forge is addressing PR review feedback now. This status update is informational.

ekuris-redhat · 2026-07-30T17:16:57Z

Forge is addressing PR review feedback now. This status update is informational.

ekuris-redhat · 2026-07-30T17:37:45Z

Forge is addressing PR review feedback now. This status update is informational.

ekuris-redhat · 2026-07-30T17:38:14Z

Forge is addressing PR review feedback now. This status update is informational.

ekuris-redhat

Hey, a few things I'd like addressed before we move forward with this one.

First, and most importantly: the .mypy_cache/3.11/ directory got committed — 18 binary cache files are showing up in the diff.
These are local mypy artifacts that should never be tracked. Please remove all of them from the branch and also add
.mypy_cache/ to .gitignore since it's currently missing from there.

Duplicate code in the worker: In worker.py, the block that finds the original review comment and edits it appears twice in a
row — once in the forge command handler and once in the revision comment handler. These are nearly identical. Please extract
that logic into a small helper so we don't have to maintain two copies.

format_review_comment duplication: The stories and tasks branches in draft_manager.py are about 95% identical. The only things
that differ are the header text, the field label, and the approval label. Please collapse them into a single helper that
accepts those as parameters.

Unnecessary LangChain wrapping: In revise_draft_with_feedback, load_prompt already returns a fully formatted string. Wrapping
it in PromptTemplate.from_template("{prompt_text}") just to pass it through is extra ceremony that doesn't add anything. A
direct call to model.ainvoke(prompt_text) would do the same thing with fewer moving parts.

A few smaller things:

The nested from datetime import datetime inside the worker function body should move to the top of the file alongside the
existing from datetime import UTC.

available_repos: Any = set() in epic_decomposition.py should use the concrete type set[str] instead of Any.

In get_attachments, each attachment dict gets both a content and a content_url key pointing to the same value. One is enough.

The _validate_item_params method in DraftManager manually replicates the type checks that Pydantic's DraftItem.model_validate
would already enforce. Consider leaning on Pydantic for this instead.

Finally, provision_epics_from_draft and provision_tasks_from_draft are called from both the worker and the route functions. The
guard on epic_keys prevents double execution, but the split ownership is confusing. Please add a comment explaining why both
call sites need to exist, or consolidate them.

ekuris-redhat · 2026-07-30T19:11:35Z

Forge is addressing PR review feedback now. This status update is informational.

ekuris-redhat · 2026-07-30T19:12:34Z

Forge is addressing PR review feedback now. This status update is informational.

ekuris-redhat

A few concrete issues I want to flag before merging:

format_review_comment ignores the module's own constants

draft_manager.py defines FORGE_STORIES_DRAFT_FILENAME and FORGE_TASKS_DRAFT_FILENAME at module level, but format_review_comment
hardcodes the literal strings "forge-stories-draft.json" and "forge-tasks-draft.json" in two places. If the filenames ever
change, this method will silently diverge. Replace with the constants.

getattr(settings, "yolo_mode", False) is stale — the field exists

This PR adds yolo_mode: bool = Field(default=False) to config.py, which means the getattr fallback in epic_decomposition.py and
task_generation.py is unnecessary. Use settings.yolo_mode directly.

Duplicate ValidationError import inside _validate_item_params

from pydantic import ValidationError is already imported at the top of draft_manager.py. The same import inside
_validate_item_params's body is redundant — remove it.

YOLO detection copy-pasted 4 times

The same three-component check — "forge:yolo" in labels or getattr(settings, "yolo_mode", False) or state.get("yolo_mode",
False) — appears identically in epic_decomposition.py, task_generation.py, plan_approval.py, and task_approval.py. This should
be a shared helper so all four sites stay in sync.

provision_epics_from_draft and provision_tasks_from_draft use Any for typed parameters

Both functions are declared (state: Any, jira: Any). They should use WorkflowState and JiraClient.

ekuris-redhat · 2026-07-30T19:59:10Z

Forge is addressing PR review feedback now. This status update is informational.

ekuris-redhat · 2026-07-30T20:00:13Z

Forge is addressing PR review feedback now. This status update is informational.

ekuris-redhat

Please address the following issues found in the review:

Unguarded split outside the non-blocking try block (update_docs_repo.py:65)

current_repo.split("/", 1) runs before the try block that starts at line 100. If current_repo is empty or missing a /, the
unpacking raises ValueError that is not caught by the non-blocking handler at line 197 — breaking the non-blocking guarantee.
Add a guard before line 65:

if not current_repo or "/" not in current_repo:
logger.warning(f"current_repo is missing or malformed for {ticket_key}, skipping docs repo update")
return state

import subprocess inside function body (update_docs_repo.py:258)

Move the import subprocess statement to the top of the module with the other imports.

Bare except Exception silently swallows errors in _branch_has_commits (update_docs_repo.py:269)

When the git command fails for any reason, the function silently returns False, causing PR creation to be skipped even when the
container committed documentation changes. Add a log statement:

except Exception as e:
logger.warning(f"Could not check for commits in {workspace_path}: {e}")
return False

_branch_has_commits bypasses GitOperations abstraction (update_docs_repo.py:256-270)

All other git operations in this codebase go through GitOperations. Move this logic into a GitOperations method (e.g.,
has_commits_ahead(base_branch: str) -> bool) and call it through docs_git instead of using subprocess directly.

Duplicate default-branch fetch pattern (update_docs_repo.py:69-90)

Lines 69-77 and 82-90 are identical: create a GitHubClient, call get_repository, extract default_branch, log, close. Extract to
a shared helper:

async def _get_repo_default_branch(settings: Settings, owner: str, repo_name: str) -> str:
github = GitHubClient(settings)
try:
data = await github.get_repository(owner, repo_name)
return data.get("default_branch", "main")
except Exception as e:
logger.warning(f"Could not fetch default branch for {owner}/{repo_name}, defaulting to 'main': {e}")
return "main"
finally:
await github.close()

Dead branch in guardrails slice (update_docs_repo.py:160)

guardrails is already defaulted to "" at line 92 via .get("guardrails", ""). The if guardrails else "" branch is unreachable —
""[:2000] and "" are identical. Simplify to guardrails[:2000].

Missing tests

Add tests for:

current_repo empty or missing / — verifies the guard added in item 1 returns state without crashing
GitError fallback path (lines 121-145) — branch deleted after merge, code falls back to fetching merge commit SHA via
get_pull_request
_create_docs_pr directly — verify fork creation, fork sync, push, PR creation, and Jira comment are all called with the
correct arguments

ekuris-redhat · 2026-08-02T08:41:58Z

Forge is addressing PR review feedback now. This status update is informational.

ekuris-redhat · 2026-08-02T08:43:03Z

Forge is addressing PR review feedback now. This status update is informational.

ekuris-redhat

Two issues that need addressing:

JSON boundary extraction uses wrong end delimiter (src/forge/integrations/agents/agent.py)

When the LLM response has no markdown code block, the fallback uses max(rfind("}"), rfind("]")) to find the end of the JSON.
This is wrong when the types are mismatched — if the JSON starts with { but there's a trailing ] after the closing } (common in
LLM responses with postamble), max picks the ] and the extracted slice is invalid JSON.

Fix: match the end delimiter to the opening delimiter:

if start_idx == start_brace:
end_idx = cleaned_text.rfind("}")
else:
end_idx = cleaned_text.rfind("]")

Pipe characters in item summary or repo break the Jira markdown table
(src/forge/workflow/utils/draft_manager.py:format_review_comment)

item.summary and item.repo are interpolated directly into table cells without escaping. A summary like "Support A | B toggle"
produces a broken 4-column row instead of 3. Escape pipe characters before interpolation:

def _escape_cell(text: str) -> str:
return text.replace("|", "\|")

table += f"| {item.id} | {_escape_cell(item.summary)} | {_escape_cell(item.repo or 'unknown')} |\n"

Apply the same escaping in the condensed table path.

ekuris-redhat · 2026-08-02T10:52:53Z

Forge is addressing PR review feedback now. This status update is informational.

ekuris-redhat · 2026-08-02T10:54:05Z

Forge is addressing PR review feedback now. This status update is informational.

eranco74

This is a great feature — exactly what I had in mind in #218. The draft review loop, /forge commands, state consistency guard, and YOLO bypass are all well thought out. A few issues to look at before merging.

eranco74 · 2026-08-02T12:07:54Z

+ 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(


Security: The raw LLM response is included in this ValueError (Response: {response}). This exception propagates to the worker where it gets posted as a Jira comment via f"Forge command/revision failed: {str(e)}". The LLM response may contain prompt internals or system instructions that shouldn't be visible to users.
Suggestion: log the full response at ERROR level but raise with a sanitized message like "Failed to parse revised draft as valid JSON".

Forge implemented this feedback in the latest pushed revision.

eranco74 · 2026-08-02T12:07:54Z

+ exc_info=True,
+ )
+
+ error_comment_text = f"❌ Forge command/revision failed: {str(e)}"


Security:str(e) for HTTP exceptions can contain request URLs, auth headers, or API tokens. Same issue on lines 1780 and 1805 for provisioning errors. Consider sanitizing or using a generic user-facing message while logging the full exception separately (which you're already doing with exc_info=True above).

Forge implemented this feedback in the latest pushed revision.

eranco74 · 2026-08-02T12:07:54Z

+ try:
+ epic_keys = await provision_epics_from_draft(state, jira)
+ # Store the newly created keys
+ state["epic_keys"] = epic_keys


Correctness: Mutating state["epic_keys"] directly inside a routing function is risky — LangGraph checkpoints state before routing, so these mutations aren't captured. If the process crashes after provision_epics_from_draft creates Jira tickets and deletes the draft but before the next node checkpoints, on restart: epic_keys won't be in the checkpoint, the draft is already deleted, and provisioning would either fail (no draft) or create duplicates.
The worker path (line ~1773) has the same pattern but at least runs outside the LangGraph graph. Consider moving provisioning entirely into the worker or into a dedicated node that checkpoints the created keys before deleting the draft.

Forge implemented this feedback in the latest pushed revision.

eranco74 · 2026-08-02T12:07:54Z

+ jira = JiraClient()
+ try:
+ epic_keys = await provision_epics_from_draft(cast(Any, updated_state), jira)
+ updated_state["epic_keys"] = epic_keys


Correctness (minor): Both label-based approval (adding forge:plan-approved) and command-based approval (/forge approve) can trigger provisioning. If both arrive as near-simultaneous webhook events, two workers could each pass the not updated_state.get("epic_keys") guard since each loads the same checkpoint independently. The window is small but could create duplicate tickets. A Jira-side guard (check if children already exist before creating) would make this idempotent.

Forge implemented this feedback in the latest pushed revision.

This feedback has already been successfully addressed on the branch via JQL-based Jira-side idempotency guards.

ekuris-redhat · 2026-08-02T12:08:32Z

Forge is addressing PR review feedback now. This status update is informational.

ekuris-redhat · 2026-08-02T12:09:37Z

Forge is addressing PR review feedback now. This status update is informational.

eranco74 · 2026-08-02T12:43:41Z

/lgtm

ekuris-redhat · 2026-08-02T12:46:37Z

/lgtm

thanks. I am now testing it and I will share the results when I have them.

forgeSmith-bot · 2026-08-18T18:33:58Z