fix(qa): use dynamic prompt injection and fix browser tool selection
Bug 1: QA reviewer was using load_qa_reviewer_prompt() instead of
get_qa_reviewer_prompt(spec_dir, project_dir). This meant QA agents
never received dynamically-injected project-specific MCP tool docs
(e.g., Electron validation for Electron apps, Puppeteer for web).
Fix:
- Import get_qa_reviewer_prompt from prompts_pkg
- Add project_dir parameter to run_qa_agent_session()
- Update loop.py to pass project_dir to reviewer
- Remove redundant session context (now included in dynamic prompt)
Bug 2: Browser tool selection in client.py didn't check for
"not is_electron" when adding Puppeteer tools. If an Electron project
had ELECTRON_MCP_ENABLED=false, the elif would incorrectly add
Puppeteer tools to the Electron app.
Fix:
- Add "and not project_capabilities.get('is_electron')" to Puppeteer
condition, matching the pattern in permissions.py:138
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.5
parent
50f739dc16
commit
a03fa8bc80
@@ -205,11 +205,14 @@ def create_client(
|
||||
# added by get_agent_allowed_tools() via _get_qa_mcp_tools() for QA agents
|
||||
|
||||
# Determine which browser automation tools to allow based on project type
|
||||
# Note: Must check "not is_electron" for Puppeteer to avoid tool mismatch
|
||||
# when Electron MCP is disabled for an Electron project
|
||||
browser_tools_permissions = []
|
||||
if agent_type in ("qa_reviewer", "qa_fixer"):
|
||||
if project_capabilities.get("is_electron") and electron_mcp_enabled:
|
||||
browser_tools_permissions = ELECTRON_TOOLS
|
||||
elif project_capabilities.get("is_web_frontend"):
|
||||
elif project_capabilities.get("is_web_frontend") and not project_capabilities.get("is_electron"):
|
||||
# Only add Puppeteer for non-Electron web frontends
|
||||
browser_tools_permissions = PUPPETEER_TOOLS
|
||||
|
||||
# Create comprehensive security settings
|
||||
|
||||
@@ -60,10 +60,7 @@ from .report import (
|
||||
)
|
||||
|
||||
# Agent sessions
|
||||
from .reviewer import (
|
||||
load_qa_reviewer_prompt,
|
||||
run_qa_agent_session,
|
||||
)
|
||||
from .reviewer import run_qa_agent_session
|
||||
|
||||
# Public API
|
||||
__all__ = [
|
||||
@@ -96,7 +93,6 @@ __all__ = [
|
||||
"_normalize_issue_key",
|
||||
"_issue_similarity",
|
||||
# Agent sessions
|
||||
"load_qa_reviewer_prompt",
|
||||
"run_qa_agent_session",
|
||||
"load_qa_fixer_prompt",
|
||||
"run_qa_fixer_session",
|
||||
|
||||
@@ -223,8 +223,13 @@ async def run_qa_validation_loop(
|
||||
async with client:
|
||||
debug("qa_loop", "Running QA reviewer agent session...")
|
||||
status, response = await run_qa_agent_session(
|
||||
client, spec_dir, qa_iteration, MAX_QA_ITERATIONS, verbose,
|
||||
previous_error=last_error_context # Pass error context for self-correction
|
||||
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
|
||||
|
||||
@@ -47,7 +47,6 @@ from qa import (
|
||||
load_implementation_plan,
|
||||
load_qa_fixer_prompt,
|
||||
# Agent sessions
|
||||
load_qa_reviewer_prompt,
|
||||
print_qa_status,
|
||||
record_iteration,
|
||||
run_qa_agent_session,
|
||||
@@ -90,7 +89,6 @@ __all__ = [
|
||||
"_normalize_issue_key",
|
||||
"_issue_similarity",
|
||||
# Agent sessions
|
||||
"load_qa_reviewer_prompt",
|
||||
"run_qa_agent_session",
|
||||
"load_qa_fixer_prompt",
|
||||
"run_qa_fixer_session",
|
||||
|
||||
+11
-24
@@ -10,6 +10,7 @@ from pathlib import Path
|
||||
|
||||
from claude_agent_sdk import ClaudeSDKClient
|
||||
from debug import debug, debug_detailed, debug_error, debug_section, debug_success
|
||||
from prompts_pkg import get_qa_reviewer_prompt
|
||||
from task_logger import (
|
||||
LogEntryType,
|
||||
LogPhase,
|
||||
@@ -18,22 +19,6 @@ from task_logger import (
|
||||
|
||||
from .criteria import get_qa_signoff_status
|
||||
|
||||
# Configuration
|
||||
QA_PROMPTS_DIR = Path(__file__).parent.parent / "prompts"
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# PROMPT LOADING
|
||||
# =============================================================================
|
||||
|
||||
|
||||
def load_qa_reviewer_prompt() -> str:
|
||||
"""Load the QA reviewer agent prompt."""
|
||||
prompt_file = QA_PROMPTS_DIR / "qa_reviewer.md"
|
||||
if not prompt_file.exists():
|
||||
raise FileNotFoundError(f"QA reviewer prompt not found: {prompt_file}")
|
||||
return prompt_file.read_text()
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# QA REVIEWER SESSION
|
||||
@@ -42,6 +27,7 @@ def load_qa_reviewer_prompt() -> str:
|
||||
|
||||
async def run_qa_agent_session(
|
||||
client: ClaudeSDKClient,
|
||||
project_dir: Path,
|
||||
spec_dir: Path,
|
||||
qa_session: int,
|
||||
max_iterations: int,
|
||||
@@ -53,6 +39,7 @@ async def run_qa_agent_session(
|
||||
|
||||
Args:
|
||||
client: Claude SDK client
|
||||
project_dir: Project root directory (for capability detection)
|
||||
spec_dir: Spec directory
|
||||
qa_session: QA iteration number
|
||||
max_iterations: Maximum number of QA iterations
|
||||
@@ -85,19 +72,19 @@ async def run_qa_agent_session(
|
||||
message_count = 0
|
||||
tool_count = 0
|
||||
|
||||
# Load QA prompt
|
||||
prompt = load_qa_reviewer_prompt()
|
||||
# Load QA prompt with dynamically-injected project-specific MCP tools
|
||||
# This includes Electron validation for Electron apps, Puppeteer for web, etc.
|
||||
prompt = get_qa_reviewer_prompt(spec_dir, project_dir)
|
||||
debug_detailed(
|
||||
"qa_reviewer", "Loaded QA reviewer prompt", prompt_length=len(prompt)
|
||||
"qa_reviewer",
|
||||
"Loaded QA reviewer prompt with project-specific tools",
|
||||
prompt_length=len(prompt),
|
||||
project_dir=str(project_dir),
|
||||
)
|
||||
|
||||
# Add session context - use full path so agent can find files
|
||||
# Add session context
|
||||
prompt += f"\n\n---\n\n**QA Session**: {qa_session}\n"
|
||||
prompt += f"**Spec Directory**: {spec_dir}\n"
|
||||
prompt += f"**Spec Name**: {spec_dir.name}\n"
|
||||
prompt += f"**Max Iterations**: {max_iterations}\n"
|
||||
prompt += f"\n**IMPORTANT**: All spec files (spec.md, implementation_plan.json, etc.) are located in: `{spec_dir}/`\n"
|
||||
prompt += f"Use the full path when reading files, e.g.: `cat {spec_dir}/spec.md`\n"
|
||||
|
||||
# Add error context for self-correction if previous iteration failed
|
||||
if previous_error:
|
||||
|
||||
@@ -24,7 +24,6 @@ from qa import (
|
||||
load_implementation_plan,
|
||||
load_qa_fixer_prompt,
|
||||
# Agent sessions
|
||||
load_qa_reviewer_prompt,
|
||||
print_qa_status,
|
||||
record_iteration,
|
||||
run_qa_agent_session,
|
||||
@@ -66,7 +65,6 @@ __all__ = [
|
||||
"_normalize_issue_key",
|
||||
"_issue_similarity",
|
||||
# Agent sessions
|
||||
"load_qa_reviewer_prompt",
|
||||
"run_qa_agent_session",
|
||||
"load_qa_fixer_prompt",
|
||||
"run_qa_fixer_session",
|
||||
|
||||
Reference in New Issue
Block a user