Files
Aperant/apps/backend/qa/loop.py
T
souky-byte 9734b70b05 chore: Refactor/kanban realtime status sync (#249)
* fix(execution): add structured phase event emission and improve phase transition handling

- Add emit_phase() calls in coder.py for PLANNING, CODING, COMPLETE, and FAILED phases
- Add emit_phase() calls in planner.py for follow-up planning phase
- Add emit_phase() calls in qa/loop.py for QA_REVIEW, QA_FIXING, COMPLETE, and FAILED phases
- Add parsePhaseEvent() to agent-events.ts to prioritize structured events over log parsing
- Default to 'planning' phase in PhaseProgressIndicator when running but phase

* fix(execution): prevent premature 'complete' phase during QA workflow

- Remove COMPLETE phase emission from coder.py when subtasks finish (QA hasn't run yet)
- Add phase regression prevention in agent-events.ts to block fallback text matching from moving backwards (e.g., QA → coding)
- Remove 'complete' phase detection from "BUILD COMPLETE" banner text (only structured emit_phase(COMPLETE) from QA approval should set complete)
- Add line buffering in agent-process.ts to prevent split __EXEC_PHASE

* fix(execution): prevent phase regression and improve JSON parsing robustness

- Add phase regression check to 'planning' phase detection in agent-events.ts
- Prevent 'failed' phase from overwriting 'complete' or 'failed' from structured events in agent-process.ts
- Add extractJsonObject() to handle JSON with trailing garbage in phase-event-parser.ts
  - Implements brace-matching parser that handles escaped quotes and nested objects
  - Prevents parse failures when __EXEC_PHASE__ JSON is followed by log

* refactor: improve variable naming clarity in phase event parsing

- Rename 'escape' to 'isEscaped' in extractJsonObject() for better readability
- Rename list comprehension variable 'l' to 'line' in test_phase_event.py

* feat(agent-events): add Zod validation and refactor phase event parsing

- Add zod dependency (^4.2.1) for runtime type validation
- Refactor phase event parsing into specialized parser classes:
  - ExecutionPhaseParser for task execution phases
  - IdeationPhaseParser for ideation workflow phases
  - RoadmapPhaseParser for roadmap generation phases
- Add strict Zod schemas for phase event validation:
  - Reject invalid message types (must be string)
  - Reject invalid progress values (must be 0-100

* refactor(agent-events): remove parser delegation and inline phase detection logic

- Remove ExecutionPhaseParser, IdeationPhaseParser, and RoadmapPhaseParser delegation
- Inline all phase detection logic directly into AgentEvents methods
- Add wouldPhaseRegress() check to prevent fallback text matching from moving backwards
- Add parsePhaseEvent() call to prioritize structured __EXEC_PHASE__ events
- Add checkRegression() helper to validate phase transitions before applying fallback matches
- Filter

* test(subprocess): update test expectations to include newlines in buffered output

- Update subprocess-spawn.test.ts to append '\n' to test data and expectations
- Reflects line buffering behavior where output is processed line-by-line
- Skip ipc-handlers.test.ts exit event test (status change logic removed)
- Remove exit code 0 test case that no longer applies after status change removal

* refactor(ideation-phase-parser): add terminal state guard and extract progress calculation

- Add terminal state check to prevent phase changes after completion
- Extract calculateGeneratingProgress() helper with division-by-zero protection
- Return 90% progress fallback when totalTypes is 0 or negative
- Apply helper to both progress calculation paths (no phase change and phase detected)

* fix(phase-parser): prevent premature QA phase detection during planning

Add canEnterQAPhase guard to fallback text matching in agent-events.ts
and execution-phase-parser.ts. QA phases can now only be triggered via
text matching if currentPhase is already 'coding', 'qa_review', or
'qa_fixing'. This prevents tasks from jumping to QA Review column when
planning phase output contains QA-related text.

Structured events from backend (__EXEC_PHASE__:...) bypass this check.

* fix(task-store): prevent stale plan data from overriding status during active execution

When a task is restarted, the file watcher immediately reads the existing
implementation_plan.json and calls updateTaskFromPlan. If the old plan has
all subtasks completed, it would set status to 'ai_review' before the agent
process emits the 'planning' phase event.

This fix checks if the task is in an active execution phase (planning,
coding, qa_review, qa_fixing) and if so, does NOT let the plan data
override the status. The execution phase takes precedence.

Added 4 tests to verify the behavior.

* Reorder imports in coder.py for clarity

Moved the import of ExecutionPhase and emit_phase from phase_event to follow the project's import organization conventions and improve code readability.

* fix: address PR review comments from CodeRabbit

- Clamp progress values to 0-100 range in phase_event.py
- Remove unused PhaseEvent import in test file
- Simplify terminal phase check in ideation-phase-parser.ts
- Add regression prevention in roadmap-phase-parser.ts
- Use z.infer for PhaseEvent type derivation

* fix: add type assertions for Zod-validated phase values

TypeScript couldn't infer the literal type from Zod enum validation.
Added explicit type assertions since the phase is already validated.

* fix: correct misleading test name for QA loop transition

The test was named 'should not regress' but actually verified that
qa_fixing → qa_review IS allowed (valid re-review after fix).
Renamed to clarify the expected behavior.

* fix: define phase variable from rawPhase in PhaseProgressIndicator

The prop was renamed during destructuring but the derived variable
was never defined, causing 'phase is not defined' runtime error.

* fix(security): add Python path validation to prevent command injection

Add validatePythonPath() function that validates user-configurable Python
paths before use in spawn(). This prevents potential command injection
attacks via malicious paths.

Security checks implemented:
- Block shell metacharacters (;|&<> etc.)
- Validate against allowlist of known Python locations
- Verify file exists and is executable
- Confirm it's actually Python via --version

Applied validation to all affected locations:
- AgentProcessManager.configure()
- InsightsConfig.configure()
- ChangelogService.configure()
- TitleGenerator.configure()

Addresses: PR #249 review - CRITICAL security finding

* fix: add sequence tracking to prevent race conditions in state updates

Add sequenceNumber field to ExecutionProgress to track update order and
prevent stale updates from overwriting newer state.

Changes:
- Add sequenceNumber to ExecutionProgress interface
- updateExecutionProgress now rejects updates with lower sequence numbers
- All execution-progress emissions now include monotonically increasing
  sequence numbers

This prevents race conditions where out-of-order updates could cause
incorrect task state display.

Addresses: PR #249 review - HIGH severity race condition finding

* refactor: extract helper methods from spawnProcess() to reduce complexity

Break down the 294-line spawnProcess() into smaller focused methods:
- setupProcessEnvironment(): Creates the process environment object
- handleProcessFailure(): Orchestrates rate limit and auth failure handling
- handleRateLimitWithAutoSwap(): Handles auto-swap logic for rate limits
- handleAuthFailure(): Detects and handles authentication failures

The main spawnProcess() is now significantly cleaner with single-responsibility
helper methods that are easier to test and maintain.

Addresses: PR #249 review - HIGH severity complexity finding

* fix: improve phase handling with type guards and better error reporting

- Add type guard validation in checkRegression() before calling
  wouldPhaseRegress() to prevent undefined lookups in PHASE_ORDER_INDEX
- Add warning log when calculateOverallProgress() receives unknown phase
  instead of silently returning 0%
- Change 'failed' phase index from 5 to 99 to clearly indicate it's
  outside normal progression (like 'idle' uses -1)

These changes improve defensive programming and debugging capabilities
for phase state management.

Addresses: PR #249 review - MEDIUM severity findings

* refactor(security): consolidate Python path validation logic into reusable helper

Extract repeated validation pattern into getValidatedPythonPath() helper to reduce code duplication across services.

Changes:
- Add getValidatedPythonPath() helper that encapsulates validation logic
- Replace duplicated validation blocks in ChangelogService, InsightsConfig, and TitleGenerator with helper call
- Improve isSafePythonCommand() to normalize whitespace before checking
- Add newline/carriage return to DANGEROUS_SHELL_CHARS regex

* fix(tests): enable exit event forwarding test

- Remove it.skip from 'should forward exit events with status change on failure'
- Add proper test setup: create project and task before emitting exit event
- Add mock for notificationService to prevent errors during test

* fix(security): use mkdtempSync for secure temp directory in tests

Addresses CodeQL 'Insecure temporary file' warning by using
mkdtempSync with a random suffix instead of a predictable path.

---------

Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com>
Co-authored-by: Alex <63423455+AlexMadera@users.noreply.github.com>
2025-12-27 15:21:35 +01:00

525 lines
19 KiB
Python

"""
QA Validation Loop Orchestration
=================================
Main QA loop that coordinates reviewer and fixer sessions until
approval or max iterations.
"""
import time as time_module
from pathlib import Path
from core.client import create_client
from debug import debug, debug_error, debug_section, debug_success, debug_warning
from linear_updater import (
LinearTaskState,
is_linear_enabled,
linear_qa_approved,
linear_qa_max_iterations,
linear_qa_rejected,
linear_qa_started,
)
from phase_config import get_phase_model, get_phase_thinking_budget
from phase_event import ExecutionPhase, emit_phase
from progress import count_subtasks, is_build_complete
from task_logger import (
LogPhase,
get_task_logger,
)
from .criteria import (
get_qa_iteration_count,
get_qa_signoff_status,
is_qa_approved,
)
from .fixer import run_qa_fixer_session
from .report import (
create_manual_test_plan,
escalate_to_human,
get_iteration_history,
get_recurring_issue_summary,
has_recurring_issues,
is_no_test_project,
record_iteration,
)
from .reviewer import run_qa_agent_session
# Configuration
MAX_QA_ITERATIONS = 50
MAX_CONSECUTIVE_ERRORS = 3 # Stop after 3 consecutive errors without progress
# =============================================================================
# QA VALIDATION LOOP
# =============================================================================
async def run_qa_validation_loop(
project_dir: Path,
spec_dir: Path,
model: str,
verbose: bool = False,
) -> bool:
"""
Run the full QA validation loop.
This is the self-validating loop:
1. QA Agent reviews
2. If rejected → Fixer Agent fixes
3. QA Agent re-reviews
4. Loop until approved or max iterations
Enhanced with:
- Iteration tracking with detailed history
- Recurring issue detection (3+ occurrences → human escalation)
- No-test project handling
Args:
project_dir: Project root directory
spec_dir: Spec directory
model: Claude model to use
verbose: Whether to show detailed output
Returns:
True if QA approved, False otherwise
"""
debug_section("qa_loop", "QA Validation Loop")
debug(
"qa_loop",
"Starting QA validation loop",
project_dir=str(project_dir),
spec_dir=str(spec_dir),
model=model,
max_iterations=MAX_QA_ITERATIONS,
)
print("\n" + "=" * 70)
print(" QA VALIDATION LOOP")
print(" Self-validating quality assurance")
print("=" * 70)
# Initialize task logger for the validation phase
task_logger = get_task_logger(spec_dir)
# Verify build is complete
if not is_build_complete(spec_dir):
debug_warning("qa_loop", "Build is not complete, cannot run QA")
print("\n❌ Build is not complete. Cannot run QA validation.")
completed, total = count_subtasks(spec_dir)
debug("qa_loop", "Build progress", completed=completed, total=total)
print(f" Progress: {completed}/{total} subtasks completed")
return False
# Emit phase event at start of QA validation (before any early returns)
emit_phase(ExecutionPhase.QA_REVIEW, "Starting QA validation")
# Check if there's pending human feedback that needs to be processed
fix_request_file = spec_dir / "QA_FIX_REQUEST.md"
has_human_feedback = fix_request_file.exists()
# Check if already approved - but if there's human feedback, we need to process it first
if is_qa_approved(spec_dir) and not has_human_feedback:
debug_success("qa_loop", "Build already approved by QA")
print("\n✅ Build already approved by QA.")
return True
# If there's human feedback, we need to run the fixer first before re-validating
if has_human_feedback:
debug(
"qa_loop",
"Human feedback detected - will run fixer first",
fix_request_file=str(fix_request_file),
)
emit_phase(ExecutionPhase.QA_FIXING, "Processing human feedback")
print("\n📝 Human feedback detected. Running QA Fixer first...")
# Get model and thinking budget for fixer (uses QA phase config)
qa_model = get_phase_model(spec_dir, "qa", model)
fixer_thinking_budget = get_phase_thinking_budget(spec_dir, "qa")
fix_client = create_client(
project_dir,
spec_dir,
qa_model,
agent_type="qa_fixer",
max_thinking_tokens=fixer_thinking_budget,
)
async with fix_client:
fix_status, fix_response = await run_qa_fixer_session(
fix_client,
spec_dir,
0,
False, # iteration 0 for human feedback
)
if fix_status == "error":
debug_error("qa_loop", f"Fixer error: {fix_response[:200]}")
print(f"\n❌ Fixer encountered error: {fix_response}")
return False
debug_success("qa_loop", "Human feedback fixes applied")
print("\n✅ Fixes applied based on human feedback. Running QA validation...")
# Remove the fix request file after processing
try:
fix_request_file.unlink()
debug("qa_loop", "Removed processed QA_FIX_REQUEST.md")
except OSError:
pass # Ignore if file removal fails
# Check for no-test projects
if is_no_test_project(spec_dir, project_dir):
print("\n⚠️ No test framework detected in project.")
print("Creating manual test plan...")
manual_plan = create_manual_test_plan(spec_dir, spec_dir.name)
print(f"📝 Manual test plan created: {manual_plan}")
print("\nNote: Automated testing will be limited for this project.")
# Start validation phase in task logger
if task_logger:
task_logger.start_phase(LogPhase.VALIDATION, "Starting QA validation...")
# Check Linear integration status
linear_task = None
if is_linear_enabled():
linear_task = LinearTaskState.load(spec_dir)
if linear_task and linear_task.task_id:
print(f"Linear task: {linear_task.task_id}")
# Update Linear to "In Review" when QA starts
await linear_qa_started(spec_dir)
print("Linear task moved to 'In Review'")
qa_iteration = get_qa_iteration_count(spec_dir)
consecutive_errors = 0
last_error_context = None # Track error for self-correction feedback
while qa_iteration < MAX_QA_ITERATIONS:
qa_iteration += 1
iteration_start = time_module.time()
debug_section("qa_loop", f"QA Iteration {qa_iteration}")
debug(
"qa_loop",
f"Starting iteration {qa_iteration}/{MAX_QA_ITERATIONS}",
iteration=qa_iteration,
max_iterations=MAX_QA_ITERATIONS,
)
print(f"\n--- QA Iteration {qa_iteration}/{MAX_QA_ITERATIONS} ---")
emit_phase(
ExecutionPhase.QA_REVIEW, f"Running QA review iteration {qa_iteration}"
)
# Run QA reviewer with phase-specific model and thinking budget
qa_model = get_phase_model(spec_dir, "qa", model)
qa_thinking_budget = get_phase_thinking_budget(spec_dir, "qa")
debug(
"qa_loop",
"Creating client for QA reviewer session...",
model=qa_model,
thinking_budget=qa_thinking_budget,
)
client = create_client(
project_dir,
spec_dir,
qa_model,
agent_type="qa_reviewer",
max_thinking_tokens=qa_thinking_budget,
)
async with client:
debug("qa_loop", "Running QA reviewer agent session...")
status, response = await run_qa_agent_session(
client,
project_dir, # Pass project_dir for capability-based tool injection
spec_dir,
qa_iteration,
MAX_QA_ITERATIONS,
verbose,
previous_error=last_error_context, # Pass error context for self-correction
)
iteration_duration = time_module.time() - iteration_start
debug(
"qa_loop",
"QA reviewer session completed",
status=status,
duration_seconds=f"{iteration_duration:.1f}",
response_length=len(response),
)
if status == "approved":
emit_phase(ExecutionPhase.COMPLETE, "QA validation passed")
# Reset error tracking on success
consecutive_errors = 0
last_error_context = None
# Record successful iteration
debug_success(
"qa_loop",
"QA APPROVED",
iteration=qa_iteration,
duration=f"{iteration_duration:.1f}s",
)
record_iteration(spec_dir, qa_iteration, "approved", [], iteration_duration)
print("\n" + "=" * 70)
print(" ✅ QA APPROVED")
print("=" * 70)
print("\nAll acceptance criteria verified.")
print("The implementation is production-ready.")
print("\nNext steps:")
print(" 1. Review the auto-claude/* branch")
print(" 2. Create a PR and merge to main")
# End validation phase successfully
if task_logger:
task_logger.end_phase(
LogPhase.VALIDATION,
success=True,
message="QA validation passed - all criteria met",
)
# Update Linear: QA approved, awaiting human review
if linear_task and linear_task.task_id:
await linear_qa_approved(spec_dir)
print("\nLinear: Task marked as QA approved, awaiting human review")
return True
elif status == "rejected":
# Reset error tracking on valid response (rejected is a valid response)
consecutive_errors = 0
last_error_context = None
debug_warning(
"qa_loop",
"QA REJECTED",
iteration=qa_iteration,
duration=f"{iteration_duration:.1f}s",
)
print(f"\n❌ QA found issues. Iteration {qa_iteration}/{MAX_QA_ITERATIONS}")
# Get issues from QA report
qa_status = get_qa_signoff_status(spec_dir)
current_issues = qa_status.get("issues_found", []) if qa_status else []
debug(
"qa_loop",
"Issues found by QA",
issue_count=len(current_issues),
issues=current_issues[:3] if current_issues else [], # Show first 3
)
# Record rejected iteration
record_iteration(
spec_dir, qa_iteration, "rejected", current_issues, iteration_duration
)
# Check for recurring issues
history = get_iteration_history(spec_dir)
has_recurring, recurring_issues = has_recurring_issues(
current_issues, history
)
if has_recurring:
from .report import RECURRING_ISSUE_THRESHOLD
debug_error(
"qa_loop",
"Recurring issues detected - escalating to human",
recurring_count=len(recurring_issues),
threshold=RECURRING_ISSUE_THRESHOLD,
)
print(
f"\n⚠️ Recurring issues detected ({len(recurring_issues)} issue(s) appeared {RECURRING_ISSUE_THRESHOLD}+ times)"
)
print("Escalating to human review due to recurring issues...")
# Create escalation file
await escalate_to_human(spec_dir, recurring_issues, qa_iteration)
# End validation phase
if task_logger:
task_logger.end_phase(
LogPhase.VALIDATION,
success=False,
message=f"QA escalated to human after {qa_iteration} iterations due to recurring issues",
)
# Update Linear
if linear_task and linear_task.task_id:
await linear_qa_max_iterations(spec_dir, qa_iteration)
print(
"\nLinear: Task marked as needing human intervention (recurring issues)"
)
return False
# Record rejection in Linear
if linear_task and linear_task.task_id:
issues_count = len(current_issues)
await linear_qa_rejected(spec_dir, issues_count, qa_iteration)
if qa_iteration >= MAX_QA_ITERATIONS:
print("\n⚠️ Maximum QA iterations reached.")
print("Escalating to human review.")
break
# Run fixer with phase-specific thinking budget
fixer_thinking_budget = get_phase_thinking_budget(spec_dir, "qa")
debug(
"qa_loop",
"Starting QA fixer session...",
model=qa_model,
thinking_budget=fixer_thinking_budget,
)
emit_phase(ExecutionPhase.QA_FIXING, "Fixing QA issues")
print("\nRunning QA Fixer Agent...")
fix_client = create_client(
project_dir,
spec_dir,
qa_model,
agent_type="qa_fixer",
max_thinking_tokens=fixer_thinking_budget,
)
async with fix_client:
fix_status, fix_response = await run_qa_fixer_session(
fix_client, spec_dir, qa_iteration, verbose
)
debug(
"qa_loop",
"QA fixer session completed",
fix_status=fix_status,
response_length=len(fix_response),
)
if fix_status == "error":
debug_error("qa_loop", f"Fixer error: {fix_response[:200]}")
print(f"\n❌ Fixer encountered error: {fix_response}")
record_iteration(
spec_dir,
qa_iteration,
"error",
[{"title": "Fixer error", "description": fix_response}],
)
break
debug_success("qa_loop", "Fixes applied, re-running QA validation")
print("\n✅ Fixes applied. Re-running QA validation...")
elif status == "error":
consecutive_errors += 1
debug_error(
"qa_loop",
f"QA session error: {response[:200]}",
consecutive_errors=consecutive_errors,
max_consecutive=MAX_CONSECUTIVE_ERRORS,
)
print(f"\n❌ QA error: {response}")
print(
f" Consecutive errors: {consecutive_errors}/{MAX_CONSECUTIVE_ERRORS}"
)
record_iteration(
spec_dir,
qa_iteration,
"error",
[{"title": "QA error", "description": response}],
)
# Build error context for self-correction in next iteration
last_error_context = {
"error_type": "missing_implementation_plan_update",
"error_message": response,
"consecutive_errors": consecutive_errors,
"expected_action": "You MUST update implementation_plan.json with a qa_signoff object containing 'status': 'approved' or 'status': 'rejected'",
"file_path": str(spec_dir / "implementation_plan.json"),
}
# Check if we've hit max consecutive errors
if consecutive_errors >= MAX_CONSECUTIVE_ERRORS:
debug_error(
"qa_loop",
f"Max consecutive errors ({MAX_CONSECUTIVE_ERRORS}) reached - escalating to human",
)
print(
f"\n⚠️ {MAX_CONSECUTIVE_ERRORS} consecutive errors without progress."
)
print(
"The QA agent is unable to properly update implementation_plan.json."
)
print("Escalating to human review.")
# End validation phase as failed
if task_logger:
task_logger.end_phase(
LogPhase.VALIDATION,
success=False,
message=f"QA agent failed {MAX_CONSECUTIVE_ERRORS} consecutive times - unable to update implementation_plan.json",
)
return False
print("Retrying with error feedback...")
# Max iterations reached without approval
emit_phase(ExecutionPhase.FAILED, "QA validation incomplete")
debug_error(
"qa_loop",
"QA VALIDATION INCOMPLETE - max iterations reached",
iterations=qa_iteration,
max_iterations=MAX_QA_ITERATIONS,
)
print("\n" + "=" * 70)
print(" ⚠️ QA VALIDATION INCOMPLETE")
print("=" * 70)
print(f"\nReached maximum iterations ({MAX_QA_ITERATIONS}) without approval.")
print("\nRemaining issues require human review:")
# Show iteration summary
history = get_iteration_history(spec_dir)
summary = get_recurring_issue_summary(history)
debug(
"qa_loop",
"QA loop final summary",
total_iterations=len(history),
total_issues=summary.get("total_issues", 0),
unique_issues=summary.get("unique_issues", 0),
)
if summary["total_issues"] > 0:
print("\n📊 Iteration Summary:")
print(f" Total iterations: {len(history)}")
print(f" Total issues found: {summary['total_issues']}")
print(f" Unique issues: {summary['unique_issues']}")
if summary.get("most_common"):
print(" Most common issues:")
for issue in summary["most_common"][:3]:
print(f" - {issue['title']} ({issue['occurrences']} occurrences)")
# End validation phase as failed
if task_logger:
task_logger.end_phase(
LogPhase.VALIDATION,
success=False,
message=f"QA validation incomplete after {qa_iteration} iterations",
)
# Show the fix request file if it exists
fix_request_file = spec_dir / "QA_FIX_REQUEST.md"
if fix_request_file.exists():
print(f"\nSee: {fix_request_file}")
qa_report_file = spec_dir / "qa_report.md"
if qa_report_file.exists():
print(f"See: {qa_report_file}")
# Update Linear: max iterations reached, needs human intervention
if linear_task and linear_task.task_id:
await linear_qa_max_iterations(spec_dir, qa_iteration)
print("\nLinear: Task marked as needing human intervention")
print("\nManual intervention required.")
return False