fix(memory): fix learning loop to retrieve patterns and gotchas (#530)
* perf: fix frontend lag with batched IPC events and optimized store updates Critical performance fixes addressing 2-5s UI lag during task execution: Frontend optimizations: - Batch IPC log events (100+/sec → 6/sec batched updates) - Add batchAppendLogs to task-store for efficient log appending - Only set updatedAt on phase changes, not every progress tick - Memoize sanitizeMarkdownForDisplay and formatRelativeTime in TaskCard - Add React.memo with custom comparators to DroppableColumn - Use IntersectionObserver to pause animations when cards not visible - Reduce debug logging verbosity Backend optimizations: - Add project index caching with 5-minute TTL in client.py - Batch StatusManager file writes with threading.Timer debounce 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore(deps): update all frontend dependencies including major versions Updates all frontend dependencies to latest versions: - react-resizable-panels 3.0.6 → 4.2.0 (breaking API change) - globals 16.5.0 → 17.0.0 - lucide-react 0.560.0 → 0.562.0 - zod 4.2.1 → 4.3.4 - Plus other minor/patch updates Updates TerminalGrid.tsx for react-resizable-panels v4 API: - PanelGroup → Group - PanelResizeHandle → Separator - direction → orientation - Removed order prop - Changed div wrappers to React.Fragment for proper resize handling 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(memory): fix learning loop to retrieve patterns and gotchas The memory system was storing patterns and gotchas correctly (100% working) but never retrieving them for agent prompts. The root cause was that get_relevant_context() only performed generic semantic search without filtering for specific episode types. Changes: - Add get_patterns_and_gotchas() method to search.py that specifically retrieves PATTERN and GOTCHA episodes with focused queries - Add min_score filtering to reduce noise from low-relevance results - Add wrapper method to graphiti.py facade class - Update memory_manager.py to call new method and format results into dedicated "Learned Patterns" and "Known Gotchas" sections This enables cross-session learning where patterns discovered in session 1 will now be available to sessions 2, 3, 4, etc. * memory is now a app wide setting * fix(security): address PR review findings for cache and subprocess safety Fixes the following issues from PR review: HIGH severity: - Return defensive copies from _get_cached_project_data() to prevent cache corruption when callers modify returned dictionaries - Validate head_sha before subprocess calls using _validate_git_ref() to prevent command injection attacks MEDIUM severity: - Add timeout=120 to worktree add subprocess call - Add timeout=30 to worktree remove/prune subprocess calls - Add bounds checking to worktree list parsing to prevent IndexError - Validate head_sha fallback to head_branch (catches invalid refs early) - Add AttributeError to exception handling in search.py JSON parsing FALSE POSITIVES (already implemented): - QA fixer/reviewer memory context - both files already have get_graphiti_context() calls at lines 106 and 92 respectively --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Andy
@@ -257,16 +257,33 @@ async def run_autonomous_agent(
|
||||
phase_thinking_budget = get_phase_thinking_budget(spec_dir, current_phase)
|
||||
|
||||
# Create client (fresh context) with phase-specific model and thinking
|
||||
# Use appropriate agent_type for correct tool permissions and thinking budget
|
||||
client = create_client(
|
||||
project_dir,
|
||||
spec_dir,
|
||||
phase_model,
|
||||
agent_type="planner" if first_run else "coder",
|
||||
max_thinking_tokens=phase_thinking_budget,
|
||||
)
|
||||
|
||||
# Generate appropriate prompt
|
||||
if first_run:
|
||||
prompt = generate_planner_prompt(spec_dir, project_dir)
|
||||
|
||||
# Retrieve Graphiti memory context for planning phase
|
||||
# This gives the planner knowledge of previous patterns, gotchas, and insights
|
||||
planner_context = await get_graphiti_context(
|
||||
spec_dir,
|
||||
project_dir,
|
||||
{
|
||||
"description": "Planning implementation for new feature",
|
||||
"id": "planner",
|
||||
},
|
||||
)
|
||||
if planner_context:
|
||||
prompt += "\n\n" + planner_context
|
||||
print_status("Graphiti memory context loaded for planner", "success")
|
||||
|
||||
first_run = False
|
||||
current_log_phase = LogPhase.PLANNING
|
||||
|
||||
|
||||
@@ -146,6 +146,12 @@ async def get_graphiti_context(
|
||||
# Get relevant context
|
||||
context_items = await memory.get_relevant_context(query, num_results=5)
|
||||
|
||||
# Get patterns and gotchas specifically (THE FIX for learning loop!)
|
||||
# This retrieves PATTERN and GOTCHA episode types for cross-session learning
|
||||
patterns, gotchas = await memory.get_patterns_and_gotchas(
|
||||
query, num_results=3, min_score=0.5
|
||||
)
|
||||
|
||||
# Also get recent session history
|
||||
session_history = await memory.get_session_history(limit=3)
|
||||
|
||||
@@ -156,10 +162,12 @@ async def get_graphiti_context(
|
||||
"memory",
|
||||
"Graphiti context retrieval complete",
|
||||
context_items_found=len(context_items) if context_items else 0,
|
||||
patterns_found=len(patterns) if patterns else 0,
|
||||
gotchas_found=len(gotchas) if gotchas else 0,
|
||||
session_history_found=len(session_history) if session_history else 0,
|
||||
)
|
||||
|
||||
if not context_items and not session_history:
|
||||
if not context_items and not session_history and not patterns and not gotchas:
|
||||
if is_debug_enabled():
|
||||
debug("memory", "No relevant context found in Graphiti")
|
||||
return None
|
||||
@@ -175,6 +183,34 @@ async def get_graphiti_context(
|
||||
item_type = item.get("type", "unknown")
|
||||
sections.append(f"- **[{item_type}]** {content}\n")
|
||||
|
||||
# Add patterns section (cross-session learning)
|
||||
if patterns:
|
||||
sections.append("### Learned Patterns\n")
|
||||
sections.append("_Patterns discovered in previous sessions:_\n")
|
||||
for p in patterns:
|
||||
pattern_text = p.get("pattern", "")
|
||||
applies_to = p.get("applies_to", "")
|
||||
if applies_to:
|
||||
sections.append(
|
||||
f"- **Pattern**: {pattern_text}\n _Applies to:_ {applies_to}\n"
|
||||
)
|
||||
else:
|
||||
sections.append(f"- **Pattern**: {pattern_text}\n")
|
||||
|
||||
# Add gotchas section (cross-session learning)
|
||||
if gotchas:
|
||||
sections.append("### Known Gotchas\n")
|
||||
sections.append("_Pitfalls to avoid:_\n")
|
||||
for g in gotchas:
|
||||
gotcha_text = g.get("gotcha", "")
|
||||
solution = g.get("solution", "")
|
||||
if solution:
|
||||
sections.append(
|
||||
f"- **Gotcha**: {gotcha_text}\n _Solution:_ {solution}\n"
|
||||
)
|
||||
else:
|
||||
sections.append(f"- **Gotcha**: {gotcha_text}\n")
|
||||
|
||||
if session_history:
|
||||
sections.append("### Recent Session Insights\n")
|
||||
for session in session_history[:2]: # Only show last 2
|
||||
|
||||
@@ -12,6 +12,7 @@ The client factory now uses AGENT_CONFIGS from agents/tools_pkg/models.py as the
|
||||
single source of truth for phase-aware tool and MCP server configuration.
|
||||
"""
|
||||
|
||||
import copy
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
@@ -61,7 +62,8 @@ def _get_cached_project_data(
|
||||
f"[ClientCache] Cache HIT for project index (age: {cache_age:.1f}s / TTL: {_CACHE_TTL_SECONDS}s)"
|
||||
)
|
||||
logger.debug(f"Using cached project index for {project_dir}")
|
||||
return cached_index, cached_capabilities
|
||||
# Return deep copies to prevent callers from corrupting the cache
|
||||
return copy.deepcopy(cached_index), copy.deepcopy(cached_capabilities)
|
||||
elif debug:
|
||||
print(
|
||||
f"[ClientCache] Cache EXPIRED for project index (age: {cache_age:.1f}s > TTL: {_CACHE_TTL_SECONDS}s)"
|
||||
@@ -91,10 +93,12 @@ def _get_cached_project_data(
|
||||
print(
|
||||
"[ClientCache] Cache was populated by another thread, using cached data"
|
||||
)
|
||||
return cached_index, cached_capabilities
|
||||
# Return deep copies to prevent callers from corrupting the cache
|
||||
return copy.deepcopy(cached_index), copy.deepcopy(cached_capabilities)
|
||||
# Either no cache entry or it's expired - store our fresh data
|
||||
_PROJECT_INDEX_CACHE[key] = (project_index, project_capabilities, time.time())
|
||||
|
||||
# Return the freshly loaded data (no need to copy since it's not from cache)
|
||||
return project_index, project_capabilities
|
||||
|
||||
|
||||
|
||||
@@ -343,6 +343,34 @@ class GraphitiMemory:
|
||||
|
||||
return await self._search.get_similar_task_outcomes(task_description, limit)
|
||||
|
||||
async def get_patterns_and_gotchas(
|
||||
self,
|
||||
query: str,
|
||||
num_results: int = 5,
|
||||
min_score: float = 0.5,
|
||||
) -> tuple[list[dict], list[dict]]:
|
||||
"""
|
||||
Get patterns and gotchas relevant to the query.
|
||||
|
||||
This method specifically retrieves PATTERN and GOTCHA episode types
|
||||
to enable cross-session learning. Unlike get_relevant_context(),
|
||||
it filters for these specific types rather than doing generic search.
|
||||
|
||||
Args:
|
||||
query: Search query (task description)
|
||||
num_results: Max results per type
|
||||
min_score: Minimum relevance score (0.0-1.0)
|
||||
|
||||
Returns:
|
||||
Tuple of (patterns, gotchas) lists
|
||||
"""
|
||||
if not await self._ensure_initialized():
|
||||
return [], []
|
||||
|
||||
return await self._search.get_patterns_and_gotchas(
|
||||
query, num_results, min_score
|
||||
)
|
||||
|
||||
# Status and utility methods
|
||||
|
||||
def get_status_summary(self) -> dict:
|
||||
|
||||
@@ -10,6 +10,8 @@ import logging
|
||||
from pathlib import Path
|
||||
|
||||
from .schema import (
|
||||
EPISODE_TYPE_GOTCHA,
|
||||
EPISODE_TYPE_PATTERN,
|
||||
EPISODE_TYPE_SESSION_INSIGHT,
|
||||
EPISODE_TYPE_TASK_OUTCOME,
|
||||
MAX_CONTEXT_RESULTS,
|
||||
@@ -55,6 +57,7 @@ class GraphitiSearch:
|
||||
query: str,
|
||||
num_results: int = MAX_CONTEXT_RESULTS,
|
||||
include_project_context: bool = True,
|
||||
min_score: float = 0.0,
|
||||
) -> list[dict]:
|
||||
"""
|
||||
Search for relevant context based on a query.
|
||||
@@ -104,6 +107,12 @@ class GraphitiSearch:
|
||||
}
|
||||
)
|
||||
|
||||
# Filter by minimum score if specified
|
||||
if min_score > 0:
|
||||
context_items = [
|
||||
item for item in context_items if item.get("score", 0) >= min_score
|
||||
]
|
||||
|
||||
logger.info(
|
||||
f"Found {len(context_items)} relevant context items for: {query[:50]}..."
|
||||
)
|
||||
@@ -153,7 +162,7 @@ class GraphitiSearch:
|
||||
):
|
||||
continue
|
||||
sessions.append(data)
|
||||
except (json.JSONDecodeError, TypeError):
|
||||
except (json.JSONDecodeError, TypeError, AttributeError):
|
||||
continue
|
||||
|
||||
# Sort by session number and return latest
|
||||
@@ -205,7 +214,7 @@ class GraphitiSearch:
|
||||
"score": getattr(result, "score", 0.0),
|
||||
}
|
||||
)
|
||||
except (json.JSONDecodeError, TypeError):
|
||||
except (json.JSONDecodeError, TypeError, AttributeError):
|
||||
continue
|
||||
|
||||
return outcomes[:limit]
|
||||
@@ -213,3 +222,107 @@ class GraphitiSearch:
|
||||
except Exception as e:
|
||||
logger.warning(f"Failed to get similar task outcomes: {e}")
|
||||
return []
|
||||
|
||||
async def get_patterns_and_gotchas(
|
||||
self,
|
||||
query: str,
|
||||
num_results: int = 5,
|
||||
min_score: float = 0.5,
|
||||
) -> tuple[list[dict], list[dict]]:
|
||||
"""
|
||||
Retrieve patterns and gotchas relevant to the current task.
|
||||
|
||||
Unlike get_relevant_context(), this specifically filters for
|
||||
EPISODE_TYPE_PATTERN and EPISODE_TYPE_GOTCHA episodes to enable
|
||||
cross-session learning.
|
||||
|
||||
Args:
|
||||
query: Search query (task description)
|
||||
num_results: Max results per type
|
||||
min_score: Minimum relevance score (0.0-1.0)
|
||||
|
||||
Returns:
|
||||
Tuple of (patterns, gotchas) lists
|
||||
"""
|
||||
patterns = []
|
||||
gotchas = []
|
||||
|
||||
try:
|
||||
# Search with query focused on patterns
|
||||
pattern_results = await self.client.graphiti.search(
|
||||
query=f"pattern: {query}",
|
||||
group_ids=[self.group_id],
|
||||
num_results=num_results * 2,
|
||||
)
|
||||
|
||||
for result in pattern_results:
|
||||
content = getattr(result, "content", None) or getattr(
|
||||
result, "fact", None
|
||||
)
|
||||
score = getattr(result, "score", 0.0)
|
||||
|
||||
if score < min_score:
|
||||
continue
|
||||
|
||||
if content and EPISODE_TYPE_PATTERN in str(content):
|
||||
try:
|
||||
data = (
|
||||
json.loads(content) if isinstance(content, str) else content
|
||||
)
|
||||
if data.get("type") == EPISODE_TYPE_PATTERN:
|
||||
patterns.append(
|
||||
{
|
||||
"pattern": data.get("pattern", ""),
|
||||
"applies_to": data.get("applies_to", ""),
|
||||
"example": data.get("example", ""),
|
||||
"score": score,
|
||||
}
|
||||
)
|
||||
except (json.JSONDecodeError, TypeError, AttributeError):
|
||||
continue
|
||||
|
||||
# Search with query focused on gotchas
|
||||
gotcha_results = await self.client.graphiti.search(
|
||||
query=f"gotcha pitfall avoid: {query}",
|
||||
group_ids=[self.group_id],
|
||||
num_results=num_results * 2,
|
||||
)
|
||||
|
||||
for result in gotcha_results:
|
||||
content = getattr(result, "content", None) or getattr(
|
||||
result, "fact", None
|
||||
)
|
||||
score = getattr(result, "score", 0.0)
|
||||
|
||||
if score < min_score:
|
||||
continue
|
||||
|
||||
if content and EPISODE_TYPE_GOTCHA in str(content):
|
||||
try:
|
||||
data = (
|
||||
json.loads(content) if isinstance(content, str) else content
|
||||
)
|
||||
if data.get("type") == EPISODE_TYPE_GOTCHA:
|
||||
gotchas.append(
|
||||
{
|
||||
"gotcha": data.get("gotcha", ""),
|
||||
"trigger": data.get("trigger", ""),
|
||||
"solution": data.get("solution", ""),
|
||||
"score": score,
|
||||
}
|
||||
)
|
||||
except (json.JSONDecodeError, TypeError, AttributeError):
|
||||
continue
|
||||
|
||||
# Sort by score and limit
|
||||
patterns.sort(key=lambda x: x.get("score", 0), reverse=True)
|
||||
gotchas.sort(key=lambda x: x.get("score", 0), reverse=True)
|
||||
|
||||
logger.info(
|
||||
f"Found {len(patterns)} patterns and {len(gotchas)} gotchas for: {query[:50]}..."
|
||||
)
|
||||
return patterns[:num_results], gotchas[:num_results]
|
||||
|
||||
except Exception as e:
|
||||
logger.warning(f"Failed to get patterns/gotchas: {e}")
|
||||
return [], []
|
||||
|
||||
@@ -3,10 +3,16 @@ QA Fixer Agent Session
|
||||
=======================
|
||||
|
||||
Runs QA fixer sessions to resolve issues identified by the reviewer.
|
||||
|
||||
Memory Integration:
|
||||
- Retrieves past patterns, fixes, and gotchas before fixing
|
||||
- Saves fix outcomes and learnings after session
|
||||
"""
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
# Memory integration for cross-session learning
|
||||
from agents.memory_manager import get_graphiti_context, save_session_memory
|
||||
from claude_agent_sdk import ClaudeSDKClient
|
||||
from debug import debug, debug_detailed, debug_error, debug_section, debug_success
|
||||
from security.tool_input_validator import get_safe_tool_input
|
||||
@@ -45,6 +51,7 @@ async def run_qa_fixer_session(
|
||||
spec_dir: Path,
|
||||
fix_session: int,
|
||||
verbose: bool = False,
|
||||
project_dir: Path | None = None,
|
||||
) -> tuple[str, str]:
|
||||
"""
|
||||
Run a QA fixer agent session.
|
||||
@@ -54,12 +61,18 @@ async def run_qa_fixer_session(
|
||||
spec_dir: Spec directory
|
||||
fix_session: Fix iteration number
|
||||
verbose: Whether to show detailed output
|
||||
project_dir: Project root directory (for memory context)
|
||||
|
||||
Returns:
|
||||
(status, response_text) where status is:
|
||||
- "fixed" if fixes were applied
|
||||
- "error" if an error occurred
|
||||
"""
|
||||
# Derive project_dir from spec_dir if not provided
|
||||
# spec_dir is typically: /project/.auto-claude/specs/001-name/
|
||||
if project_dir is None:
|
||||
# Walk up from spec_dir to find project root
|
||||
project_dir = spec_dir.parent.parent.parent
|
||||
debug_section("qa_fixer", f"QA Fixer Session {fix_session}")
|
||||
debug(
|
||||
"qa_fixer",
|
||||
@@ -89,6 +102,20 @@ async def run_qa_fixer_session(
|
||||
prompt = load_qa_fixer_prompt()
|
||||
debug_detailed("qa_fixer", "Loaded QA fixer prompt", prompt_length=len(prompt))
|
||||
|
||||
# Retrieve memory context for fixer (past fixes, patterns, gotchas)
|
||||
fixer_memory_context = await get_graphiti_context(
|
||||
spec_dir,
|
||||
project_dir,
|
||||
{
|
||||
"description": "Fixing QA issues and implementing corrections",
|
||||
"id": f"qa_fixer_{fix_session}",
|
||||
},
|
||||
)
|
||||
if fixer_memory_context:
|
||||
prompt += "\n\n" + fixer_memory_context
|
||||
print("✓ Memory context loaded for QA fixer")
|
||||
debug_success("qa_fixer", "Graphiti memory context loaded for fixer")
|
||||
|
||||
# Add session context - use full path so agent can find files
|
||||
prompt += f"\n\n---\n\n**Fix Session**: {fix_session}\n"
|
||||
prompt += f"**Spec Directory**: {spec_dir}\n"
|
||||
@@ -244,12 +271,42 @@ async def run_qa_fixer_session(
|
||||
if status
|
||||
else False,
|
||||
)
|
||||
|
||||
# Save fixer session insights to memory
|
||||
fixer_discoveries = {
|
||||
"files_understood": {},
|
||||
"patterns_found": [
|
||||
f"QA fixer session {fix_session}: Applied fixes from QA_FIX_REQUEST.md"
|
||||
],
|
||||
"gotchas_encountered": [],
|
||||
}
|
||||
|
||||
if status and status.get("ready_for_qa_revalidation"):
|
||||
debug_success("qa_fixer", "Fixes applied, ready for QA revalidation")
|
||||
# Save successful fix session to memory
|
||||
await save_session_memory(
|
||||
spec_dir=spec_dir,
|
||||
project_dir=project_dir,
|
||||
subtask_id=f"qa_fixer_{fix_session}",
|
||||
session_num=fix_session,
|
||||
success=True,
|
||||
subtasks_completed=[f"qa_fixer_{fix_session}"],
|
||||
discoveries=fixer_discoveries,
|
||||
)
|
||||
return "fixed", response_text
|
||||
else:
|
||||
# Fixer didn't update the status properly, but we'll trust it worked
|
||||
debug_success("qa_fixer", "Fixes assumed applied (status not updated)")
|
||||
# Still save to memory as successful (fixes were attempted)
|
||||
await save_session_memory(
|
||||
spec_dir=spec_dir,
|
||||
project_dir=project_dir,
|
||||
subtask_id=f"qa_fixer_{fix_session}",
|
||||
session_num=fix_session,
|
||||
success=True,
|
||||
subtasks_completed=[f"qa_fixer_{fix_session}"],
|
||||
discoveries=fixer_discoveries,
|
||||
)
|
||||
return "fixed", response_text
|
||||
|
||||
except Exception as e:
|
||||
|
||||
@@ -4,10 +4,16 @@ QA Reviewer Agent Session
|
||||
|
||||
Runs QA validation sessions to review implementation against
|
||||
acceptance criteria.
|
||||
|
||||
Memory Integration:
|
||||
- Retrieves past patterns, gotchas, and insights before QA session
|
||||
- Saves QA findings (bugs, patterns, validation outcomes) after session
|
||||
"""
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
# Memory integration for cross-session learning
|
||||
from agents.memory_manager import get_graphiti_context, save_session_memory
|
||||
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
|
||||
@@ -82,6 +88,20 @@ async def run_qa_agent_session(
|
||||
project_dir=str(project_dir),
|
||||
)
|
||||
|
||||
# Retrieve memory context for QA (past patterns, gotchas, validation insights)
|
||||
qa_memory_context = await get_graphiti_context(
|
||||
spec_dir,
|
||||
project_dir,
|
||||
{
|
||||
"description": "QA validation and acceptance criteria review",
|
||||
"id": f"qa_reviewer_{qa_session}",
|
||||
},
|
||||
)
|
||||
if qa_memory_context:
|
||||
prompt += "\n\n" + qa_memory_context
|
||||
print("✓ Memory context loaded for QA reviewer")
|
||||
debug_success("qa_reviewer", "Graphiti memory context loaded for QA")
|
||||
|
||||
# Add session context
|
||||
prompt += f"\n\n---\n\n**QA Session**: {qa_session}\n"
|
||||
prompt += f"**Max Iterations**: {max_iterations}\n"
|
||||
@@ -307,11 +327,48 @@ This is attempt {previous_error.get("consecutive_errors", 1) + 1}. If you fail t
|
||||
response_length=len(response_text),
|
||||
qa_status=status.get("status") if status else "unknown",
|
||||
)
|
||||
|
||||
# Save QA session insights to memory
|
||||
qa_discoveries = {
|
||||
"files_understood": {},
|
||||
"patterns_found": [],
|
||||
"gotchas_encountered": [],
|
||||
}
|
||||
|
||||
if status and status.get("status") == "approved":
|
||||
debug_success("qa_reviewer", "QA APPROVED")
|
||||
qa_discoveries["patterns_found"].append(
|
||||
f"QA session {qa_session}: All acceptance criteria validated successfully"
|
||||
)
|
||||
# Save successful QA session to memory
|
||||
await save_session_memory(
|
||||
spec_dir=spec_dir,
|
||||
project_dir=project_dir,
|
||||
subtask_id=f"qa_reviewer_{qa_session}",
|
||||
session_num=qa_session,
|
||||
success=True,
|
||||
subtasks_completed=[f"qa_reviewer_{qa_session}"],
|
||||
discoveries=qa_discoveries,
|
||||
)
|
||||
return "approved", response_text
|
||||
elif status and status.get("status") == "rejected":
|
||||
debug_error("qa_reviewer", "QA REJECTED")
|
||||
# Extract issues found for memory
|
||||
issues = status.get("issues_found", [])
|
||||
for issue in issues:
|
||||
qa_discoveries["gotchas_encountered"].append(
|
||||
f"QA Issue ({issue.get('type', 'unknown')}): {issue.get('title', 'No title')} at {issue.get('location', 'unknown')}"
|
||||
)
|
||||
# Save rejected QA session to memory (learning from failures)
|
||||
await save_session_memory(
|
||||
spec_dir=spec_dir,
|
||||
project_dir=project_dir,
|
||||
subtask_id=f"qa_reviewer_{qa_session}",
|
||||
session_num=qa_session,
|
||||
success=False,
|
||||
subtasks_completed=[],
|
||||
discoveries=qa_discoveries,
|
||||
)
|
||||
return "rejected", response_text
|
||||
else:
|
||||
# Agent didn't update the status properly - provide detailed error
|
||||
|
||||
@@ -31,7 +31,7 @@ from claude_agent_sdk import AgentDefinition
|
||||
try:
|
||||
from ...core.client import create_client
|
||||
from ...phase_config import get_thinking_budget
|
||||
from ..context_gatherer import PRContext
|
||||
from ..context_gatherer import PRContext, _validate_git_ref
|
||||
from ..models import (
|
||||
GitHubRunnerConfig,
|
||||
MergeVerdict,
|
||||
@@ -43,7 +43,7 @@ try:
|
||||
from .pydantic_models import ParallelOrchestratorResponse
|
||||
from .sdk_utils import process_sdk_stream
|
||||
except (ImportError, ValueError, SystemError):
|
||||
from context_gatherer import PRContext
|
||||
from context_gatherer import PRContext, _validate_git_ref
|
||||
from core.client import create_client
|
||||
from models import (
|
||||
GitHubRunnerConfig,
|
||||
@@ -126,7 +126,7 @@ class ParallelOrchestratorReviewer:
|
||||
"""Create a temporary worktree at the PR head commit.
|
||||
|
||||
Args:
|
||||
head_sha: The commit SHA of the PR head
|
||||
head_sha: The commit SHA of the PR head (validated before use)
|
||||
pr_number: The PR number for naming
|
||||
|
||||
Returns:
|
||||
@@ -134,7 +134,15 @@ class ParallelOrchestratorReviewer:
|
||||
|
||||
Raises:
|
||||
RuntimeError: If worktree creation fails
|
||||
ValueError: If head_sha fails validation (command injection prevention)
|
||||
"""
|
||||
# SECURITY: Validate git ref before use in subprocess calls
|
||||
if not _validate_git_ref(head_sha):
|
||||
raise ValueError(
|
||||
f"Invalid git ref: '{head_sha}'. "
|
||||
"Must contain only alphanumeric characters, dots, slashes, underscores, and hyphens."
|
||||
)
|
||||
|
||||
worktree_name = f"pr-{pr_number}-{uuid.uuid4().hex[:8]}"
|
||||
worktree_dir = self.project_dir / PR_WORKTREE_DIR
|
||||
|
||||
@@ -178,6 +186,7 @@ class ParallelOrchestratorReviewer:
|
||||
cwd=self.project_dir,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=120, # Worktree add can be slow for large repos
|
||||
)
|
||||
|
||||
if DEBUG_MODE:
|
||||
@@ -239,6 +248,7 @@ class ParallelOrchestratorReviewer:
|
||||
cwd=self.project_dir,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=30,
|
||||
)
|
||||
|
||||
if DEBUG_MODE:
|
||||
@@ -258,6 +268,7 @@ class ParallelOrchestratorReviewer:
|
||||
["git", "worktree", "prune"],
|
||||
cwd=self.project_dir,
|
||||
capture_output=True,
|
||||
timeout=30,
|
||||
)
|
||||
logger.warning(f"[PRReview] Used shutil fallback for: {worktree_path.name}")
|
||||
except Exception as e:
|
||||
@@ -275,12 +286,15 @@ class ParallelOrchestratorReviewer:
|
||||
cwd=self.project_dir,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=30,
|
||||
)
|
||||
registered = {
|
||||
Path(line.split(" ", 1)[1])
|
||||
for line in result.stdout.split("\n")
|
||||
if line.startswith("worktree ")
|
||||
}
|
||||
registered = set()
|
||||
for line in result.stdout.split("\n"):
|
||||
if line.startswith("worktree "):
|
||||
# Safely parse - check bounds to prevent IndexError
|
||||
parts = line.split(" ", 1)
|
||||
if len(parts) > 1 and parts[1]:
|
||||
registered.add(Path(parts[1]))
|
||||
|
||||
# Remove unregistered directories
|
||||
stale_count = 0
|
||||
@@ -295,6 +309,7 @@ class ParallelOrchestratorReviewer:
|
||||
["git", "worktree", "prune"],
|
||||
cwd=self.project_dir,
|
||||
capture_output=True,
|
||||
timeout=30,
|
||||
)
|
||||
if DEBUG_MODE:
|
||||
print(
|
||||
@@ -618,6 +633,15 @@ The SDK will run invoked agents in parallel automatically.
|
||||
)
|
||||
print(f"[PRReview] DEBUG: resolved head_sha='{head_sha}'", flush=True)
|
||||
|
||||
# SECURITY: Validate the resolved head_sha (whether SHA or branch name)
|
||||
# This catches invalid refs early before subprocess calls
|
||||
if head_sha and not _validate_git_ref(head_sha):
|
||||
logger.warning(
|
||||
f"[ParallelOrchestrator] Invalid git ref '{head_sha}', "
|
||||
"using current checkout for safety"
|
||||
)
|
||||
head_sha = None
|
||||
|
||||
if not head_sha:
|
||||
if DEBUG_MODE:
|
||||
print("[PRReview] DEBUG: No head_sha - using fallback", flush=True)
|
||||
@@ -647,7 +671,7 @@ The SDK will run invoked agents in parallel automatically.
|
||||
f"project_root={project_root}",
|
||||
flush=True,
|
||||
)
|
||||
except RuntimeError as e:
|
||||
except (RuntimeError, ValueError) as e:
|
||||
if DEBUG_MODE:
|
||||
print(
|
||||
f"[PRReview] DEBUG: Worktree creation FAILED: {e}",
|
||||
|
||||
@@ -11,6 +11,9 @@ import { projectStore } from '../project-store';
|
||||
import { getClaudeProfileManager } from '../claude-profile-manager';
|
||||
import { parsePythonCommand, validatePythonPath } from '../python-detector';
|
||||
import { pythonEnvManager, getConfiguredPythonPath } from '../python-env-manager';
|
||||
import { buildMemoryEnvVars } from '../memory-env-builder';
|
||||
import { readSettingsFile } from '../settings-utils';
|
||||
import type { AppSettings } from '../../shared/types/settings';
|
||||
|
||||
/**
|
||||
* Process spawning and lifecycle management
|
||||
@@ -574,14 +577,24 @@ export class AgentProcessManager {
|
||||
* Get combined environment variables for a project
|
||||
*
|
||||
* Priority (later sources override earlier):
|
||||
* 1. Backend source .env (apps/backend/.env) - CLI defaults
|
||||
* 2. Project's .auto-claude/.env - Frontend-configured settings (memory, integrations)
|
||||
* 3. Project settings (graphitiMcpUrl, useClaudeMd) - Runtime overrides
|
||||
* 1. App-wide memory settings from settings.json (NEW - enables memory from onboarding)
|
||||
* 2. Backend source .env (apps/backend/.env) - CLI defaults
|
||||
* 3. Project's .auto-claude/.env - Frontend-configured settings (memory, integrations)
|
||||
* 4. Project settings (graphitiMcpUrl, useClaudeMd) - Runtime overrides
|
||||
*/
|
||||
getCombinedEnv(projectPath: string): Record<string, string> {
|
||||
// Load app-wide memory settings from settings.json
|
||||
// This bridges onboarding config to backend agents
|
||||
const appSettings = (readSettingsFile() || {}) as Partial<AppSettings>;
|
||||
const memoryEnv = buildMemoryEnvVars(appSettings as AppSettings);
|
||||
|
||||
// Existing env sources
|
||||
const autoBuildEnv = this.loadAutoBuildEnv();
|
||||
const projectFileEnv = this.loadProjectEnv(projectPath);
|
||||
const projectSettingsEnv = this.getProjectEnvVars(projectPath);
|
||||
return { ...autoBuildEnv, ...projectFileEnv, ...projectSettingsEnv };
|
||||
|
||||
// Priority: app-wide memory -> backend .env -> project .env -> project settings
|
||||
// Later sources override earlier ones
|
||||
return { ...memoryEnv, ...autoBuildEnv, ...projectFileEnv, ...projectSettingsEnv };
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,6 +12,9 @@ import {
|
||||
validateEmbeddingConfiguration,
|
||||
getGraphitiDatabaseDetails
|
||||
} from './utils';
|
||||
import { buildMemoryEnvVars } from '../../memory-env-builder';
|
||||
import { readSettingsFile } from '../../settings-utils';
|
||||
import type { AppSettings } from '../../../shared/types/settings';
|
||||
|
||||
/**
|
||||
* Load Graphiti state from most recent spec directory
|
||||
@@ -54,18 +57,31 @@ export function loadGraphitiStateFromSpecs(
|
||||
|
||||
/**
|
||||
* Build memory status from environment configuration
|
||||
*
|
||||
* Priority (same as agent-process.ts getCombinedEnv):
|
||||
* 1. App-wide memory settings from settings.json (from onboarding)
|
||||
* 2. Project's .env files
|
||||
*/
|
||||
export function buildMemoryStatus(
|
||||
projectPath: string,
|
||||
autoBuildPath?: string,
|
||||
memoryState?: GraphitiMemoryState | null
|
||||
): GraphitiMemoryStatus {
|
||||
// Load app-wide memory settings from settings.json (set during onboarding)
|
||||
const appSettings = (readSettingsFile() || {}) as Partial<AppSettings>;
|
||||
const memoryEnvVars = buildMemoryEnvVars(appSettings as AppSettings);
|
||||
|
||||
// Load project-specific env vars
|
||||
const projectEnvVars = loadProjectEnvVars(projectPath, autoBuildPath);
|
||||
const globalSettings = loadGlobalSettings();
|
||||
|
||||
// Merge: app-wide memory settings -> project env vars
|
||||
// Project settings can override app-wide settings
|
||||
const effectiveEnvVars = { ...memoryEnvVars, ...projectEnvVars };
|
||||
|
||||
// If we have initialized state from specs, use it
|
||||
if (memoryState?.initialized) {
|
||||
const dbDetails = getGraphitiDatabaseDetails(projectEnvVars);
|
||||
const dbDetails = getGraphitiDatabaseDetails(effectiveEnvVars);
|
||||
return {
|
||||
enabled: true,
|
||||
available: true,
|
||||
@@ -74,9 +90,9 @@ export function buildMemoryStatus(
|
||||
};
|
||||
}
|
||||
|
||||
// Check environment configuration
|
||||
const graphitiEnabled = isGraphitiEnabled(projectEnvVars);
|
||||
const embeddingValidation = validateEmbeddingConfiguration(projectEnvVars, globalSettings);
|
||||
// Check environment configuration using merged env vars
|
||||
const graphitiEnabled = isGraphitiEnabled(effectiveEnvVars);
|
||||
const embeddingValidation = validateEmbeddingConfiguration(effectiveEnvVars, globalSettings);
|
||||
|
||||
if (!graphitiEnabled) {
|
||||
return {
|
||||
@@ -94,7 +110,7 @@ export function buildMemoryStatus(
|
||||
};
|
||||
}
|
||||
|
||||
const dbDetails = getGraphitiDatabaseDetails(projectEnvVars);
|
||||
const dbDetails = getGraphitiDatabaseDetails(effectiveEnvVars);
|
||||
return {
|
||||
enabled: true,
|
||||
available: true,
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
/**
|
||||
* Memory Environment Variable Builder
|
||||
*
|
||||
* Converts app-wide memory settings from settings.json into environment variables
|
||||
* that can be injected into Python agent processes.
|
||||
*
|
||||
* This bridges the gap between frontend settings storage and backend configuration.
|
||||
*/
|
||||
|
||||
import type { AppSettings } from '../shared/types/settings';
|
||||
|
||||
/**
|
||||
* Build environment variables for memory/Graphiti configuration from app settings.
|
||||
*
|
||||
* @param settings - App-wide settings from settings.json
|
||||
* @returns Record of environment variables to inject into agent processes
|
||||
*/
|
||||
export function buildMemoryEnvVars(settings: AppSettings): Record<string, string> {
|
||||
const env: Record<string, string> = {};
|
||||
|
||||
// If memory is not enabled, return empty env
|
||||
if (!settings.memoryEnabled) {
|
||||
return env;
|
||||
}
|
||||
|
||||
// Enable Graphiti
|
||||
env.GRAPHITI_ENABLED = 'true';
|
||||
|
||||
// Set embedder provider (default to ollama)
|
||||
const embeddingProvider = settings.memoryEmbeddingProvider || 'ollama';
|
||||
env.GRAPHITI_EMBEDDER_PROVIDER = embeddingProvider;
|
||||
|
||||
// Provider-specific configuration
|
||||
switch (embeddingProvider) {
|
||||
case 'ollama':
|
||||
env.OLLAMA_BASE_URL = settings.ollamaBaseUrl || 'http://localhost:11434';
|
||||
if (settings.memoryOllamaEmbeddingModel) {
|
||||
env.OLLAMA_EMBEDDING_MODEL = settings.memoryOllamaEmbeddingModel;
|
||||
}
|
||||
if (settings.memoryOllamaEmbeddingDim) {
|
||||
env.OLLAMA_EMBEDDING_DIM = String(settings.memoryOllamaEmbeddingDim);
|
||||
}
|
||||
break;
|
||||
|
||||
case 'openai':
|
||||
if (settings.globalOpenAIApiKey) {
|
||||
env.OPENAI_API_KEY = settings.globalOpenAIApiKey;
|
||||
}
|
||||
break;
|
||||
|
||||
case 'voyage':
|
||||
if (settings.memoryVoyageApiKey) {
|
||||
env.VOYAGE_API_KEY = settings.memoryVoyageApiKey;
|
||||
}
|
||||
break;
|
||||
|
||||
case 'google':
|
||||
if (settings.globalGoogleApiKey) {
|
||||
env.GOOGLE_API_KEY = settings.globalGoogleApiKey;
|
||||
}
|
||||
break;
|
||||
|
||||
case 'azure_openai':
|
||||
if (settings.memoryAzureApiKey) {
|
||||
env.AZURE_OPENAI_API_KEY = settings.memoryAzureApiKey;
|
||||
}
|
||||
if (settings.memoryAzureBaseUrl) {
|
||||
env.AZURE_OPENAI_BASE_URL = settings.memoryAzureBaseUrl;
|
||||
}
|
||||
if (settings.memoryAzureEmbeddingDeployment) {
|
||||
env.AZURE_OPENAI_EMBEDDING_DEPLOYMENT = settings.memoryAzureEmbeddingDeployment;
|
||||
}
|
||||
break;
|
||||
|
||||
case 'openrouter':
|
||||
if (settings.globalOpenRouterApiKey) {
|
||||
env.OPENROUTER_API_KEY = settings.globalOpenRouterApiKey;
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
return env;
|
||||
}
|
||||
@@ -140,27 +140,43 @@ export function MemoryStep({ onNext, onBack }: MemoryStepProps) {
|
||||
setError(null);
|
||||
|
||||
try {
|
||||
// Save the API keys to global settings
|
||||
const settingsToSave: Record<string, string | undefined> = {};
|
||||
|
||||
if (config.openaiApiKey.trim()) {
|
||||
settingsToSave.globalOpenAIApiKey = config.openaiApiKey.trim();
|
||||
}
|
||||
if (config.googleApiKey.trim()) {
|
||||
settingsToSave.globalGoogleApiKey = config.googleApiKey.trim();
|
||||
}
|
||||
if (config.ollamaBaseUrl.trim()) {
|
||||
settingsToSave.ollamaBaseUrl = config.ollamaBaseUrl.trim();
|
||||
}
|
||||
// Save complete memory configuration to global settings
|
||||
// This includes all settings needed for backend to use memory
|
||||
const settingsToSave: Record<string, string | number | boolean | undefined> = {
|
||||
// Core memory settings (CRITICAL - these were missing before)
|
||||
memoryEnabled: true,
|
||||
memoryEmbeddingProvider: config.embeddingProvider,
|
||||
memoryOllamaEmbeddingModel: config.ollamaEmbeddingModel || undefined,
|
||||
memoryOllamaEmbeddingDim: config.ollamaEmbeddingDim || undefined,
|
||||
// Ollama base URL
|
||||
ollamaBaseUrl: config.ollamaBaseUrl.trim() || undefined,
|
||||
// Global API keys (shared across features)
|
||||
globalOpenAIApiKey: config.openaiApiKey.trim() || undefined,
|
||||
globalGoogleApiKey: config.googleApiKey.trim() || undefined,
|
||||
// Provider-specific keys for memory
|
||||
memoryVoyageApiKey: config.voyageApiKey.trim() || undefined,
|
||||
memoryAzureApiKey: config.azureOpenaiApiKey.trim() || undefined,
|
||||
memoryAzureBaseUrl: config.azureOpenaiBaseUrl.trim() || undefined,
|
||||
memoryAzureEmbeddingDeployment: config.azureOpenaiEmbeddingDeployment.trim() || undefined,
|
||||
};
|
||||
|
||||
const result = await window.electronAPI.saveSettings(settingsToSave);
|
||||
|
||||
if (result?.success) {
|
||||
// Update local settings store
|
||||
const storeUpdate: Partial<Pick<AppSettings, 'globalOpenAIApiKey' | 'globalGoogleApiKey' | 'ollamaBaseUrl'>> = {};
|
||||
if (config.openaiApiKey.trim()) storeUpdate.globalOpenAIApiKey = config.openaiApiKey.trim();
|
||||
if (config.googleApiKey.trim()) storeUpdate.globalGoogleApiKey = config.googleApiKey.trim();
|
||||
if (config.ollamaBaseUrl.trim()) storeUpdate.ollamaBaseUrl = config.ollamaBaseUrl.trim();
|
||||
// Update local settings store with all memory config
|
||||
const storeUpdate: Partial<AppSettings> = {
|
||||
memoryEnabled: true,
|
||||
memoryEmbeddingProvider: config.embeddingProvider,
|
||||
memoryOllamaEmbeddingModel: config.ollamaEmbeddingModel || undefined,
|
||||
memoryOllamaEmbeddingDim: config.ollamaEmbeddingDim || undefined,
|
||||
ollamaBaseUrl: config.ollamaBaseUrl.trim() || undefined,
|
||||
globalOpenAIApiKey: config.openaiApiKey.trim() || undefined,
|
||||
globalGoogleApiKey: config.googleApiKey.trim() || undefined,
|
||||
memoryVoyageApiKey: config.voyageApiKey.trim() || undefined,
|
||||
memoryAzureApiKey: config.azureOpenaiApiKey.trim() || undefined,
|
||||
memoryAzureBaseUrl: config.azureOpenaiBaseUrl.trim() || undefined,
|
||||
memoryAzureEmbeddingDeployment: config.azureOpenaiEmbeddingDeployment.trim() || undefined,
|
||||
};
|
||||
updateSettings(storeUpdate);
|
||||
onNext();
|
||||
} else {
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
* Application settings types
|
||||
*/
|
||||
|
||||
import type { NotificationSettings } from './project';
|
||||
import type { NotificationSettings, GraphitiEmbeddingProvider } from './project';
|
||||
import type { ChangelogFormat, ChangelogAudience, ChangelogEmojiLevel } from './changelog';
|
||||
import type { SupportedLanguage } from '../constants/i18n';
|
||||
|
||||
@@ -234,9 +234,18 @@ export interface AppSettings {
|
||||
globalGoogleApiKey?: string;
|
||||
globalGroqApiKey?: string;
|
||||
globalOpenRouterApiKey?: string;
|
||||
// Graphiti LLM provider settings
|
||||
// Graphiti LLM provider settings (legacy)
|
||||
graphitiLlmProvider?: 'openai' | 'anthropic' | 'google' | 'groq' | 'ollama';
|
||||
ollamaBaseUrl?: string;
|
||||
// Memory/Graphiti configuration (app-wide, set during onboarding)
|
||||
memoryEnabled?: boolean;
|
||||
memoryEmbeddingProvider?: GraphitiEmbeddingProvider;
|
||||
memoryOllamaEmbeddingModel?: string;
|
||||
memoryOllamaEmbeddingDim?: number;
|
||||
memoryVoyageApiKey?: string;
|
||||
memoryAzureApiKey?: string;
|
||||
memoryAzureBaseUrl?: string;
|
||||
memoryAzureEmbeddingDeployment?: string;
|
||||
// Onboarding wizard completion state
|
||||
onboardingCompleted?: boolean;
|
||||
// Selected agent profile for preset model/thinking configurations
|
||||
|
||||
Reference in New Issue
Block a user