[AISOS-2294] Add decompose draft review step before Jira task creation

[ekuris-redhat]

Summary

This pull request implements an interactive, draft-based planning review flow for epic decomposition and task generation workflows. It introduces Pydantic models for draft representation, robust Jira attachment helpers and management utilities, comment commands and natural language feedback revision logic, and integrates a State Consistency Guard with rollback capabilities in the orchestrator worker. Ultimately, this allows human-in-the-loop validation, modification, and direct provisioning of generated tickets from structured draft JSONs attached to Jira issues. Additionally, it introduces post-merge documentation repository update support, enabling automated documentation PR creation on a separate docs repository when configured.

Changes

Models & Schema Verification

• Created src/forge/models/draft.py defining DraftItem (supporting an epic_key field to correctly map Tasks to parent Epics) and ForgeDecompositionDraft Pydantic v2 models, enforcing unique sequential item IDs and exclusion flags (via an excluded boolean property).
• Exported the draft models in src/forge/models/__init__.py.

Jira Integrations & Utilities

• Modified src/forge/integrations/jira/client.py to add robust Jira attachment helpers (get_attachments—which merges attachment listing and metadata fetching, download_attachment, delete_attachment, delete_attachments_by_name, edit_comment with robust retry logic, and add_attachment supporting dynamic content types) with automatic rate-limit and retry logic, resolved a concurrent uploading race condition, and added get_project_docs_repo to retrieve the forge.docs_repo Jira project property.
• Created src/forge/workflow/utils/draft_manager.py implementing DraftManager to handle JSON draft serialization, secure downloading, deserialization, single-draft attachment constraints, and transactional in-memory draft modifications (add, update, remove, exclude) while preserving table alignment by escaping pipe (|) characters.

Comment Command & Natural Language LLM Parsing

• Enhanced src/forge/workflow/utils/comment_classifier.py by adding CommentType.COMMAND and introducing regex-based comment command classification.
• Implemented parse_comment_command to extract operations and arguments for /forge add, /forge update, /forge remove, /forge exclude, and /forge approve.
• Created prompt template src/forge/prompts/v1/revision-draft.md to feed draft edits and natural language feedback into an LLM chain.
• Added revise_draft_with_feedback in src/forge/integrations/agents/agent.py to revise draft structures using LLM feedback with a clean, Markdown-stripping JSON parsing utility featuring delimiter-matched boundary extraction, sanitizing ValueError exceptions to prevent prompt internals leakage.

Orchestration & State Machine Integration

• Modified src/forge/workflow/nodes/epic_decomposition.py and src/forge/workflow/nodes/task_generation.py to conditionally bypass or enter the draft review gate (pausing at PENDING_APPROVAL, implementing an empty-draft guard in the non-YOLO path for epic decomposition, and writing markdown previews with a 32,767 character/15-item truncation threshold).
• Modified src/forge/orchestrator/worker.py to capture comments while paused, applying either /forge mutation commands or leading whitespace-tolerant ! natural language revisions on attached draft JSONs.
• Implemented State Consistency Guard (BR-006) in the worker to automatically roll back to the previously attached draft version and report failures (with redacted secrets) upon any processing errors.
• Moved ticket provisioning logic out of conditional edge routers and into dedicated standard graph nodes (provision_epics, provision_tasks), integrating JQL-based Jira-side idempotency guards in plan and task approval.
• Integrated automated Jira ticket provisioning upon approval comments (/forge approve) or transition label events (forge:plan-approved, forge:task-approved) inside the orchestrator worker, skipping excluded items and ensuring attachment deletion only after successful creation of all items.
• Added a dedicated post-merge update_docs_repo workflow node (src/forge/workflow/nodes/update_docs_repo.py) triggered after the merge step to handle cloning, dual-mounting (code repo as read-only, separate docs repo as read-write), fork-based pushing, and automated PR generation for external documentation repos when forge.docs_repo is configured.
• Initialized docs_pr_url in both Feature and Bug state defaults.

Documentation Updates

• Updated CLAUDE.md, docs/guide/labels.md, docs/guide/feature-workflow.md, and docs/developer-guide.md to reflect interactive comment commands, workflow diagrams, and state-machine characteristics.
• Updated config reference, README, bug/feature workflow guides, and workflow graphs to document the new update_docs_repo post-merge step and the forge.docs_repo Jira project property.

Implementation Notes

• Single Draft File Constraint: Ensures only one stories draft (forge-stories-draft.json) and one tasks draft (forge-tasks-draft.json) exists at any time on a ticket by systematically deleting obsolete attachments before uploading newly generated ones.
• State Consistency Guard (BR-006): Ensures that any validation or execution error during a draft modification rolls back the draft attachment to the pre-modification version, ensuring no corrupted or partial JSONs are persisted.
• Truncation Boundary (BR-003): Automatically formats a condensed markdown table instead of verbose task descriptions if the preview exceeds 32,767 characters or contains over 15 items, mitigating Jira API limits.
• YOLO Mode Bypass: Detects the forge:yolo label or global yolo_mode to skip the draft review loop and provision issues immediately.
• Documentation Repo Fallback & Safety: Safely returns unchanged state if code repo settings are malformed, falls back to origin remote checkout if fork information is absent (such as in same-repo PRs), dynamically fetches branch names instead of hardcoding "main", and falls back to checking out the merge commit SHA via the GitHub PR API if the branch has already been deleted after merge.

Testing

• Unit Testing: Thoroughly covered model validation sequences, comment classification regexes, mutation algorithms, custom mock LLM agents, draft manager behaviors, and documentation updater repository edge cases.
• Integration Testing: Created tests/workflow/test_draft_review_flow.py verifying full end-to-end flows: YOLO bypass, draft attachments, comment comment modifications, LLM-based draft revisions, truncation rules, and state rollbacks on execution failures. Migrated legacy integration tests in tests/integration/orchestrator/test_workflow_execution.py to the pluggable workflows.
• Test Locations:
  • tests/unit/models/test_draft.py
  • tests/unit/integrations/jira/test_client_attachments.py
  • tests/unit/workflow/utils/test_draft_manager.py
  • tests/unit/workflow/test_comment_classifier.py
  • tests/workflow/utils/test_comment_command.py
  • tests/unit/integrations/agents/test_agent.py
  • tests/unit/orchestrator/test_worker.py
  • tests/unit/workflow/nodes/test_task_generation.py
  • tests/workflow/test_draft_review_flow.py
  • tests/integration/orchestrator/test_workflow_execution.py
  • tests/unit/workflow/nodes/test_docs_updater.py

Related Tickets

• AISOS-2294
• AISOS-2298
• AISOS-2299
• AISOS-2300
• AISOS-2301
• AISOS-2302
• AISOS-2303
• AISOS-2304
• AISOS-2305
• AISOS-2306
• AISOS-2307
• AISOS-2308
• AISOS-2309
• AISOS-2310
  Fixed Add decompose draft review step before Jira task creation #218

---

Generated by Forge SDLC Orchestrator

[ekuris-redhat]

Please address the following issues found in the review:

1. 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

2. 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.

3. 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

4. _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.

5. 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()

6. 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].

7. 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]

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

[ekuris-redhat]

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

[ekuris-redhat]

Two issues that need addressing:

1. 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("]")

2. 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]

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

[ekuris-redhat]

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]

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".

[ekuris-redhat]

Forge implemented this feedback in the latest pushed revision.

[ekuris-redhat]

Forge implemented this feedback in the latest pushed revision.

[eranco74]

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).

[ekuris-redhat]

Forge implemented this feedback in the latest pushed revision.

[ekuris-redhat]

Forge implemented this feedback in the latest pushed revision.

[eranco74]

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.

[ekuris-redhat]

Forge implemented this feedback in the latest pushed revision.

[ekuris-redhat]

Forge implemented this feedback in the latest pushed revision.

[eranco74]

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.

[ekuris-redhat]

Forge implemented this feedback in the latest pushed revision.

[ekuris-redhat]

Forge implemented this feedback in the latest pushed revision.

[ekuris-redhat]

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

[ekuris-redhat]

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

[eranco74]

/lgtm

[ekuris-redhat]

/lgtm

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