Merge pull request #74 from Contrast-Security-OSS/AIML-233_qa_build_summary_bug

AIML-233 qa build summary bug

Author: JacobMagesHaskinsContrast

Makefile

@@ -0,0 +1,28 @@
+# Makefile for contrast-ai-smartfix-action
+#
+# Available targets:
+#   make test  - Run all tests using the test runner
+#   make lint  - Run linting via pre-push hook
+#   make help  - Show this help message
+
+.PHONY: test lint help
+
+# Run tests using the test runner script
+test:
+	@echo "Running tests..."
+	./test/run_tests.sh
+
+# Run linting via the pre-push hook
+lint:
+	@echo "Running linter..."
+	./.git/hooks/pre-push
+
+# Show help message
+help:
+	@echo "Available make targets:"
+	@echo "  test  - Run all tests using ./test/run_tests.sh"
+	@echo "  lint  - Run linting via ./.git/hooks/pre-push"
+	@echo "  help  - Show this help message"
+
+# Default target
+.DEFAULT_GOAL := help

src/main.py

@@ -32,6 +32,7 @@
 from src.utils import debug_log, log, error_exit
 from src import telemetry_handler
 from src.version_check import do_version_check
+from src.smartfix.domains.workflow.session_handler import create_session_handler, QASectionConfig
 from src.smartfix.shared.failure_categories import FailureCategory
 
 # Import domain-specific handlers
@@ -352,15 +353,19 @@ def main():  # noqa: C901
             vuln_title = vulnerability_data['vulnerabilityTitle']
             remediation_id = vulnerability_data['remediationId']
             session_id = vulnerability_data.get('sessionId')
-            previous_vuln_uuid = vuln_uuid  # Update tracking variable
-
-            # Create prompt configuration for SmartFix agent
-            prompts = PromptConfiguration.for_smartfix_agent(
-                fix_system_prompt=vulnerability_data['fixSystemPrompt'],
-                fix_user_prompt=vulnerability_data['fixUserPrompt'],
-                qa_system_prompt=vulnerability_data['qaSystemPrompt'],
-                qa_user_prompt=vulnerability_data['qaUserPrompt']
-            )
+
+            # Validate and create prompt configuration for SmartFix agent
+            try:
+                PromptConfiguration.validate_raw_prompts_data(vulnerability_data)
+                prompts = PromptConfiguration.for_smartfix_agent(
+                    fix_system_prompt=vulnerability_data['fixSystemPrompt'],
+                    fix_user_prompt=vulnerability_data['fixUserPrompt'],
+                    qa_system_prompt=vulnerability_data['qaSystemPrompt'],
+                    qa_user_prompt=vulnerability_data['qaUserPrompt']
+                )
+            except ValueError as e:
+                log(f"Error: Invalid prompts from backend: {e}", is_error=True)
+                error_exit(remediation_id, FailureCategory.GENERAL_FAILURE.value)
         else:
             # For external coding agents (GITHUB_COPILOT/CLAUDE_CODE), get vulnerability details
             log("\n::group::--- Fetching next vulnerability details from Contrast API ---")
@@ -381,12 +386,11 @@ def main():  # noqa: C901
             # Check if this is the same vulnerability UUID as the previous iteration
             if vuln_uuid == previous_vuln_uuid:
                 log(f"Error: Backend provided the same vulnerability UUID ({vuln_uuid}) as the previous iteration. This indicates a backend error.", is_warning=True)
-                error_exit(remediation_id, "DUPLICATE_VULNERABILITY_UUID")
+                error_exit(remediation_id, FailureCategory.GENERAL_FAILURE.value)
 
             vuln_title = vulnerability_data['vulnerabilityTitle']
             remediation_id = vulnerability_data['remediationId']
             session_id = None  # External agents don't use Contrast LLM sessions
-            previous_vuln_uuid = vuln_uuid  # Update tracking variable
 
             # Create prompt configuration for external agent (no prompts required)
             prompts = PromptConfiguration.for_external_agent()
@@ -414,6 +418,10 @@ def main():  # noqa: C901
         else:
             log(f"No existing OPEN or MERGED PR found for vulnerability {vuln_uuid}. Proceeding with fix attempt.")
         log("\n::endgroup::")
+
+        # Update tracking variable now that we know we're actually processing this vuln
+        previous_vuln_uuid = vuln_uuid
+
         log(f"\n\033[0;33m Selected vuln to fix: {vuln_title} \033[0m")
 
         # --- Create Common Remediation Context ---
@@ -460,13 +468,33 @@ def main():  # noqa: C901
         session = smartfix_agent.remediate(context)
 
         # Extract results from the session
-        if session.success:
-            ai_fix_summary_full = session.pr_body if session.pr_body else "Fix completed successfully"
-        else:
-            # Agent failed - handle the error
-            failure_category = session.failure_category.value if session.failure_category else FailureCategory.AGENT_FAILURE.value
-            log(f"Agent failed with reason: {failure_category}")
-            error_exit(remediation_id, failure_category)
+        session_handler = create_session_handler()
+        session_result = session_handler.handle_session_result(session)
+
+        if not session_result.should_continue:
+            # QA Agent failed to fix the build
+            log(f"Agent failed with reason: {session_result.failure_category}")
+            git_handler.cleanup_branch(new_branch_name)
+            contrast_api.notify_remediation_failed(
+                remediation_id=remediation_id,
+                failure_category=session_result.failure_category,
+                contrast_host=config.CONTRAST_HOST,
+                contrast_org_id=config.CONTRAST_ORG_ID,
+                contrast_app_id=config.CONTRAST_APP_ID,
+                contrast_auth_key=config.CONTRAST_AUTHORIZATION_KEY,
+                contrast_api_key=config.CONTRAST_API_KEY
+            )
+            continue  # Move to next vulnerability
+
+        ai_fix_summary_full = session_result.ai_fix_summary
+        # Generate QA section based on session results
+        # The SmartFix agent already handled the QA loop internally
+        qa_config = QASectionConfig(
+            skip_qa_review=config.SKIP_QA_REVIEW,
+            has_build_command=build_config.has_build_command(),
+            build_command=build_config.build_command
+        )
+        qa_section = session_handler.generate_qa_section(session, qa_config)
 
         # --- Git and GitHub Operations ---
         # All file changes from the agent (fix + QA + formatting) are uncommitted at this point
@@ -486,54 +514,6 @@ def main():  # noqa: C901
         git_handler.commit_changes(commit_message)
         log("Committed all agent changes.")
 
-        # Generate QA section based on session results
-        # The SmartFix agent already handled the QA loop internally
-        qa_section = "\n\n---\n\n## Review \n\n"
-
-        if not config.SKIP_QA_REVIEW and build_config.has_build_command():
-            # QA was expected to run - check session results
-            if session.qa_attempts > 0:  # QA was attempted
-                qa_section += f"*   **Build Run:** Yes (`{build_config.build_command}`)\n"
-                if session.success:
-                    qa_section += "*   **Final Build Status:** Success \n"
-                else:
-                    qa_section += "*   **Final Build Status:** Failure \n"
-
-                    # Check if we should skip PR creation due to QA failure
-                    log("\n--- Skipping PR creation as QA Agent failed to fix build issues ---")
-
-                    # Get failure category directly from session
-                    failure_category = session.failure_category.value if session.failure_category else FailureCategory.QA_AGENT_FAILURE.value
-
-                    # Notify the Remediation service about the failed remediation
-                    remediation_notified = contrast_api.notify_remediation_failed(
-                        remediation_id=remediation_id,
-                        failure_category=failure_category,
-                        contrast_host=config.CONTRAST_HOST,
-                        contrast_org_id=config.CONTRAST_ORG_ID,
-                        contrast_app_id=config.CONTRAST_APP_ID,
-                        contrast_auth_key=config.CONTRAST_AUTHORIZATION_KEY,
-                        contrast_api_key=config.CONTRAST_API_KEY
-                    )
-
-                    if remediation_notified:
-                        log(f"Successfully notified Remediation service about {failure_category} for remediation {remediation_id}.")
-                    else:
-                        log(f"Failed to notify Remediation service about {failure_category} for remediation {remediation_id}.", is_warning=True)
-
-                    git_handler.cleanup_branch(new_branch_name)
-                    contrast_api.send_telemetry_data()
-                    continue  # Move to the next vulnerability
-            else:
-                qa_section += "*   **Build Run:** No (BUILD_COMMAND not provided)\n"
-                qa_section += "*   **Final Build Status:** Skipped\n"
-        else:  # QA was skipped
-            qa_section = ""  # Ensure qa_section is empty if QA is skipped
-            if config.SKIP_QA_REVIEW:
-                log("QA Review was skipped based on SKIP_QA_REVIEW setting.")
-            elif not build_config.has_build_command():
-                log("QA Review was skipped as no BUILD_COMMAND was provided.")
-
         # --- Create Pull Request ---
         pr_title = git_handler.generate_pr_title(vuln_title)
         # Use the result from SmartFix agent remediation as the base PR body.

src/smartfix/domains/vulnerability/context.py

@@ -137,6 +137,32 @@ def _replace_security_test_section(self, prompt: str) -> str:
 
         return prompt
 
+    @classmethod
+    def validate_raw_prompts_data(cls, prompts_data: dict) -> bool:
+        """
+        Validate raw prompts data dictionary for SmartFix agents.
+
+        Args:
+            prompts_data: Dictionary containing prompt data from backend
+
+        Returns:
+            bool: True if valid
+
+        Raises:
+            ValueError: If prompts are invalid/missing
+        """
+        required_prompts = ['fixSystemPrompt', 'fixUserPrompt', 'qaSystemPrompt', 'qaUserPrompt']
+
+        for prompt_key in required_prompts:
+            if prompt_key not in prompts_data:
+                raise ValueError(f"Missing required prompt key: {prompt_key}")
+
+            prompt_value = prompts_data[prompt_key]
+            if not prompt_value or not prompt_value.strip():
+                raise ValueError(f"Missing or empty required prompt: {prompt_key}")
+
+        return True
+
     @classmethod
     def for_smartfix_agent(
         cls,

src/smartfix/domains/workflow/session_handler.py

@@ -0,0 +1,152 @@
+# -
+# #%L
+# Contrast AI SmartFix
+# %%
+# Copyright (C) 2025 Contrast Security, Inc.
+# %%
+# Contact: [email protected]
+# License: Commercial
+# NOTICE: This Software and the patented inventions embodied within may only be
+# used as part of Contrast Security's commercial offerings. Even though it is
+# made available through public repositories, use of this Software is subject to
+# the applicable End User Licensing Agreement found at
+# https://www.contrastsecurity.com/enduser-terms-0317a or as otherwise agreed
+# between Contrast Security and the End User. The Software may not be reverse
+# engineered, modified, repackaged, sold, redistributed or otherwise used in a
+# way not consistent with the End User License Agreement.
+# #L%
+#
+
+"""
+Session handling workflow for SmartFix agent results.
+
+This module provides object-oriented handling of agent session results,
+including QA section generation, success/failure determination, and validation.
+"""
+
+from typing import Optional
+from src.smartfix.shared.failure_categories import FailureCategory
+from src.utils import log
+
+
+class SessionResult:
+    """
+    Encapsulates the result of processing an agent session.
+
+    Attributes:
+        should_continue: Whether processing should continue to PR creation
+        failure_category: Failure category if session failed
+        ai_fix_summary: Summary for successful sessions
+    """
+
+    def __init__(self, should_continue: bool, failure_category: Optional[str] = None, ai_fix_summary: Optional[str] = None):
+        self.should_continue = should_continue
+        self.failure_category = failure_category
+        self.ai_fix_summary = ai_fix_summary
+
+
+class QASectionConfig:
+    """
+    Configuration for QA section generation.
+
+    Attributes:
+        skip_qa_review: Whether QA review was skipped by configuration
+        has_build_command: Whether a build command is available
+        build_command: The build command used
+    """
+
+    def __init__(self, skip_qa_review: bool, has_build_command: bool, build_command: str):
+        self.skip_qa_review = skip_qa_review
+        self.has_build_command = has_build_command
+        self.build_command = build_command
+
+
+class SessionHandler:
+    """
+    Handles SmartFix agent session results and generates appropriate responses.
+
+    This class encapsulates the business logic for:
+    - Determining session success/failure outcomes
+    - Generating QA sections for PR bodies
+    """
+
+    def __init__(self):
+        pass
+
+    def handle_session_result(self, session) -> SessionResult:
+        """
+        Handle session result and determine next action.
+
+        Args:
+            session: AgentSession with success, failure_category, pr_body properties
+
+        Returns:
+            SessionResult: Result indicating whether to continue processing
+        """
+        if session.success:
+            ai_fix_summary = session.pr_body if session.pr_body else "Fix completed successfully"
+            return SessionResult(should_continue=True, ai_fix_summary=ai_fix_summary)
+        else:
+            # Agent failed - determine failure category
+            failure_category = (
+                session.failure_category.value
+                if session.failure_category
+                else FailureCategory.AGENT_FAILURE.value
+            )
+            return SessionResult(should_continue=False, failure_category=failure_category)
+
+    def generate_qa_section(self, session, config: QASectionConfig) -> str:
+        """
+        Generate the QA section for PR body based on session results.
+
+        Args:
+            session: AgentSession with success, qa_attempts properties
+            config: QA section configuration
+
+        Returns:
+            str: QA section for PR body
+        """
+
+        # Note: At this point session.success must be True
+        # (failures are handled by handle_session_result earlier)
+        # Start with standard QA section header
+        qa_section = "\n\n---\n\n## Review \n\n"
+        if not config.skip_qa_review and config.has_build_command:
+            # QA was expected to run - check session results
+            if session.qa_attempts > 0:
+                # QA loop ran and eventually succeeded
+                qa_section += f"*   **Build Run:** Yes (`{config.build_command}`)\n"
+                qa_section += "*   **Final Build Status:** Success \n"
+            else:
+                # Build passed on first attempt, no QA needed
+                qa_section += f"*   **Build Run:** Yes (`{config.build_command}`)\n"
+                qa_section += "*   **Final Build Status:** Success (passed on first attempt)\n"
+        else:
+            # QA was skipped - provide empty section and log reason
+            qa_section = ""
+            self._log_qa_skip_reason(config)
+
+        return qa_section
+
+    def _log_qa_skip_reason(self, config: QASectionConfig) -> None:
+        """
+        Log the reason why QA review was skipped.
+
+        Args:
+            config: QA section configuration
+        """
+        if config.skip_qa_review:
+            log("QA Review was skipped based on SKIP_QA_REVIEW setting.")
+        elif not config.has_build_command:
+            log("QA Review was skipped as no BUILD_COMMAND was provided.")
+
+
+# Factory function for backward compatibility and easy instantiation
+def create_session_handler() -> SessionHandler:
+    """
+    Create a SessionHandler instance.
+
+    Returns:
+        SessionHandler: Configured session handler
+    """
+    return SessionHandler()