Files
Aperant/tests/test_github_bot_detection.py
T
Andy 5d8ede2331 Fix/2.7.2 beta12 (#424)
* feat(mcp): add per-project MCP server configuration

- Add mcpServers config to ProjectEnvConfig type for per-project overrides
- Update env-handlers to read/write MCP config from .auto-claude/.env
- Update backend get_required_mcp_servers() to respect project config
- Refactor AgentTools.tsx to show project-specific MCP toggles
- Move MCP Overview to Project section in sidebar navigation
- Add i18n translations for MCP server names and descriptions
- Update tests for new mcp_config parameter behavior

Users can now enable/disable Context7, Linear, Electron, and Puppeteer
MCP servers on a per-project basis. Settings are stored in each project's
.auto-claude/.env file and respected by the backend when starting agents.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(frontend): send final plan state before unwatching on task exit

The file watcher was being stopped before the final plan state could be
sent to the renderer. This caused tasks to show stale data (0/0 subtasks)
in the UI when they had actually completed successfully with subtasks.

Now the final plan is sent to the renderer before unwatching, ensuring
the UI receives the correct subtask count and completion status.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* ci(beta-release): add Flatpak packaging support for Linux builds

The Linux build was failing with "spawn flatpak ENOENT" because the
beta-release workflow was missing the Flatpak setup step that was added
to the main release workflow in #404.

Adds:
- Setup Flatpak step with flatpak-builder and required runtimes
- .flatpak to artifact uploads and validation

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* feat(ui): make kanban columns responsive to available width

Fixed-width columns wasted horizontal space on wider displays and
unnecessarily truncated task titles. Columns now grow with flex-1
while respecting min/max bounds (288-480px for tasks, 320-512px for
roadmap). Badge area also expanded slightly (160→180px) to accommodate
wider cards.

* feat(settings): add user-configurable utility agent settings

Make merge_resolver and commit_message agents configurable via the
new "Utility" feature setting in Agent Settings. Previously these
were hardcoded to Haiku with low thinking, but now users can select
their preferred model and thinking level.

Changes:
- Add utility feature key to FeatureModelConfig/FeatureThinkingConfig
- Update AgentTools.tsx to use feature settings instead of fixed
- Pass UTILITY_MODEL_ID and UTILITY_THINKING_BUDGET env vars to backend
- Backend merge_resolver and commit_message read from env vars
- Add i18n translations for utility settings

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix: remove unused agent-tools entry from sidebar navigation

This commit cleans up the Sidebar component by removing the 'agent-tools' entry from the tools navigation items, streamlining the user interface. The 'worktrees' entry remains intact, ensuring continued access to relevant features.

* fix(robustness): address PR review findings for error handling and validation

Fix 9 issues identified in PR #424 review:

Medium issues:
- Add try/except for int() conversion of UTILITY_THINKING_BUDGET env var
- Pass '0' when thinking level is 'none' to properly disable extended thinking
- Add error handling (|| exit 1) for Flatpak install commands in CI

Low issues:
- Log exceptions in load_project_mcp_config instead of silent pass
- Handle None input in _map_mcp_server_name to prevent AttributeError
- Cast mcp_config values to string before split() to handle non-string values
- Filter effectiveMcps by project-level MCP states in AgentTools
- Log JSON parse errors in getUtilitySettings for easier debugging

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* chore(ci): remove CLA workflow (using hosted CLA Assistant)

The CLA workflow was accidentally re-added by PR #254. We use the hosted
CLA Assistant service (cla-assistant.io) which handles CLA signing via
GitHub webhooks, so this workflow file is redundant and causes failing checks.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(ui): correct graphiti-memory server name in MCP filter

The switch case incorrectly used 'graphiti' instead of 'graphiti-memory'
which is the actual server ID used throughout the codebase. This caused
the filter to not properly check the graphiti MCP server enabled state.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(utility): correctly disable extended thinking when set to "none"

When utility thinking level is set to "none", the frontend was sending
'0' to the backend, which parsed it as integer 0. The SDK expects
max_thinking_tokens=None to disable extended thinking, not 0.

Frontend now sends empty string for disabled thinking, and backend
interprets empty string as None instead of falling back to 1024.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix issue with github PR checking bot detection

* feat(ui): add Claude Code CLI detection and one-click installation

Adds comprehensive Claude Code CLI integration to the frontend:

- New onboarding step to check if Claude Code is installed
- Persistent status badge in sidebar showing version status
- Version checking against npm registry with 24h cache
- One-click install/update using user's preferred terminal
- Cross-platform support (macOS, Windows, Linux) with 20+ terminals
- Warning dialog before updates to prevent data loss from killed sessions
- Automatic detection of running Claude processes with graceful termination
- Added ~/.local/bin to macOS PATH search for Claude CLI detection
- Full i18n support (English and French translations)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(ideation): close panel on dismiss and scope events by project

Two bugs fixed based on user feedback:

1. Dismiss idea panel not closing: The detail panel now calls onClose()
   after dismissing, so it closes automatically instead of staying open
   with hidden action buttons.

2. Idea regeneration affecting all projects: Added currentProjectId tracking
   to the ideation store. All IPC listeners now filter events by projectId,
   preventing cross-project state contamination when multiple projects are open.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* feat(github): add parallel orchestrator for PR reviews

Implement AI-orchestrated parallel review system using Claude Agent SDK
subagents for both initial and follow-up PR reviews.

Initial review uses 5 specialist agents:
- security-reviewer: OWASP Top 10, injection, auth issues
- quality-reviewer: complexity, duplication, error handling
- logic-reviewer: algorithm correctness, edge cases, race conditions
- codebase-fit-reviewer: naming conventions, pattern adherence
- ai-triage-reviewer: validate CodeRabbit, Cursor, Gemini comments

Follow-up review uses 3 specialist agents:
- resolution-verifier: AI-powered verification of previous findings
- new-code-reviewer: security/logic/quality checks on new code
- comment-analyzer: triage contributor and AI bot feedback

Key features:
- AI decides which agents to invoke (not programmatic rules)
- User-configurable models via frontend settings (no hardcoding)
- SDK handles parallel execution automatically
- Cross-validation boosts confidence when agents agree

Also fixes bot_detection.py import error for relative imports.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(core): add agents parameter to create_client for SDK subagents

The create_client function was missing support for the `agents` parameter
needed by the parallel orchestrator reviewers to define SDK subagents.

This enables the parallel PR review system to define specialist agents
(resolution-verifier, new-code-reviewer, comment-analyzer) that the
SDK can execute in parallel.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(security): add usedforsecurity=False to MD5 hash for finding IDs

The MD5 hash is used for generating unique finding IDs (non-security
purpose), so Bandit B324 warning is addressed by marking it explicitly.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* feat(github): add PR review logs feature and parallel orchestrator improvements

- Add PR logs feature to view AI review thinking/tool usage during analysis
- Create PRLogCollector class to capture and structure subprocess output
- Add PRLogs component with collapsible phases (context, analysis, synthesis)
- Improve parallel orchestrator and followup reviewer with better agent coordination
- Add bot detection improvements and fix benefit-of-doubt logic
- Add comprehensive tests for PR review, bot detection, and E2E flows
- Add IPC channel and browser mock for PR logs retrieval
- Add i18n translations for review logs UI

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(github): show PR review logs during AI analysis in progress

- Add logs section that appears when review is in progress, not just after completion
- Add periodic log refresh (2s interval) while review is streaming
- Add isStreaming prop to PRLogs component for live indicator
- Show "Live" badge on logs header during active review
- Show streaming status on active phases

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(github): show actual AI response content in PR review logs

- Update Python orchestrators to print AI response text preview (up to 500 chars)
- Filter out unhelpful debug messages like "Message #15: AssistantMessage"
- Add ParallelOrchestrator to log source patterns and color mapping
- Move Followup to analysis phase (not context) for better categorization

The logs now show actual AI thinking and responses instead of just message types.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(github): capture synthesis phase logs and mark all phases complete

- Add parsing for [PR Review Engine], [PR #XXX] progress, and Summary lines
- Add PR progress message pattern matching for [PR #XXX] [YY%] format
- Map PR Review Engine, Summary, and Progress sources to synthesis phase
- Update finalize() to mark pending phases as completed when review succeeds
- Add source colors for PR Review Engine (indigo) and Summary (emerald)

The synthesis phase now properly shows logs and marks as Complete.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* feat(ui): merge tools section into project and add dynamic nav filtering

Moves GitHub Issues, GitHub PRs, GitLab Issues, and GitLab MRs tabs from
the separate TOOLS section into the PROJECT section. Navigation items are
now dynamically filtered based on project settings - GitHub tabs only show
when GitHub is enabled, and GitLab tabs only show when GitLab is enabled.

This reduces visual clutter by hiding integrations that aren't configured
while consolidating all project-related navigation into a single section.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* feat(pr-review): implement strict quality gates severity system

Redesign PR review severity labels and verdict logic based on research:
- CRITICAL → "Blocker" (blocks merge)
- HIGH → "Required" (blocks merge)
- MEDIUM → "Recommended" (blocks merge - AI fixes quickly)
- LOW → "Suggestion" (optional)

Key changes:
- Medium severity findings now result in NEEDS_REVISION verdict
- Only LOW severity allows MERGE_WITH_CHANGES
- Updated all 5 verdict logic files for consistency
- Updated AI prompts with strict quality gates guidance
- Updated en/fr i18n labels with action-oriented terminology

Rationale: AI can fix code issues quickly, so be aggressive about
code quality. 95% of fixes are done by AI anyway.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* feat(mcp): add health checks and bearer token auth for custom MCP servers

Users adding HTTP MCP servers had no way to know if the server was
healthy, needed authentication, or was unreachable. This adds:

- Health status indicators (healthy/needs auth/unhealthy/checking)
- Quick connectivity check on component mount
- Manual "Test" button for full MCP protocol test
- Simple "Authentication Token" field that creates Bearer header
- URL pattern detection with helpful hints for known providers
  (GitHub, Google, Anthropic, OpenAI) with links to create tokens
- Collapsible "Advanced Headers" section for custom headers
- i18n translations for all new fields (EN/FR)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* feat(gitlab): add glab CLI detection and one-click install

When users try to use OAuth for GitLab authentication, the app now checks
if glab CLI is installed first. If not installed, it shows an inline
warning card with a one-click install button that opens the user's
preferred terminal with the appropriate install command (brew for macOS,
winget for Windows, snap/brew for Linux).

This prevents the silent failure that occurred when glab was missing,
where the OAuth flow would fail with ENOENT and leave users with a
perpetual loading spinner.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(pr-review): pass USE_CLAUDE_MD env var to subprocess

The PR review subprocess wasn't receiving the project's useClaudeMd
setting, causing it to always show "CLAUDE.md: disabled by project
settings" even when enabled in the UI.

Changes:
- Added optional `env` parameter to SubprocessOptions interface
- Updated runPythonSubprocess to merge custom env vars with filtered env
- PR handlers now pass USE_CLAUDE_MD based on project.settings.useClaudeMd

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(pr-review): improve agent invocation and findings logging

The logs previously showed "Agents invoked: []" even when agents were
running because the streaming detection wasn't reliable. Also, the
findings summary was hidden.

Changes:
- Extract agents from structured output (reliable source) instead of
  streaming detection which was returning empty
- Log each specialist agent with [Agent:name] label when complete
- Add detailed findings summary showing severity, title, file:line
- Both parallel orchestrator and followup reviewer updated

Example new log output:
  [ParallelOrchestrator] Specialist agents invoked: security-reviewer, logic-reviewer
  [Agent:security-reviewer] Analysis complete
  [Agent:logic-reviewer] Analysis complete
  [ParallelOrchestrator] Findings summary:
    [LOW] 1. Suggestion title (file.ts:42)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* refactor(pr-review): enhance quality gates and logging for PR assessments

Updated the PR review process to enforce stricter quality gates for severity levels, ensuring that HIGH and MEDIUM issues block merges. Adjusted the verdict criteria for clarity and consistency across the system. Enhanced logging for PR reviews to provide real-time updates and improved user feedback.

Changes:
- Revised verdict criteria to reflect strict quality gates
- Updated logging to capture all findings, including LOW severity suggestions
- Improved real-time log streaming during PR reviews

This ensures a more robust and user-friendly review process, emphasizing the importance of addressing all findings.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

* feat(pr-review): add real-time log streaming and specialist agent tracking

- Add incremental log saving in PRLogCollector (every 3 entries) for real-time streaming
- Add phase transition tracking to properly mark phases as complete
- Add parsing for specialist agent logs ([Agent:xxx] format)
- Add color-coded badges for specialist agents in frontend logs UI
- Track subagent invocations via ToolUseBlock/ToolResultBlock in message content

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(pr-review): improve verdict display, follow-up timing, and findings UX

Three fixes for PR review:

1. Verdict message now includes LOW findings count (e.g., "1 required, 0 recommended, 4 suggestions")

2. "Ready for Follow-up" only shows when commits happen AFTER findings are posted, not during/before the review

3. Posted findings are hidden from selection UI - shows "All findings posted to GitHub" instead of confusing "0/5 selected"

Also removes legacy orchestrator_reviewer.py (dead code superseded by parallel orchestrator)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(review): address PR #424 code review feedback

Fixes issues flagged by CodeRabbit, Cursor bot, and GitHub security:

- Fix nullish coalescing for 'none' thinking budget (worktree-handlers.ts)
- Add set -e to Flatpak CI setup for consistent error handling
- Add explicit UTF-8 encoding to file open in client.py
- Add type="button" to 10 buttons to prevent form submissions
- Remove unused isLoading and getServerStatus variables
- Improve French translations with proper definite articles

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(pr-review): ensure synthesis tab shows completed status after review

When a follow-up review completed, the Synthesis tab would continue
showing "Running" instead of "Complete". This was due to a race
condition where the log polling stopped immediately when isReviewing
became false, before fetching the final log state with completed
phase statuses.

Added a final log refresh when the review completes by tracking the
previous isReviewing state and fetching logs one more time when the
state transitions from true to false.

* fix(security): address code review security and quality issues

Fixes security vulnerabilities and code quality issues from Auto Claude review:

- Fix command injection: use execFileSync with args array instead of
  template string interpolation for git commands (worktree-handlers.ts)
- Fix JSON injection: add schema validation for CUSTOM_MCP_SERVERS to
  reject malicious configurations (client.py)
- Fix code smell: use proper destructuring for unused state variable
- Add i18n: replace hardcoded English strings with translation keys
- Extract shared utility: create core/model_config.py to eliminate
  duplicate model/thinking budget parsing code (DRY violation)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(pr-review): clear stale logs when starting new follow-up review

When starting a second follow-up review, the UI was showing the
previous review's completed phase statuses instead of fresh pending
states. This happened because the logs state wasn't cleared when a
new review started.

Now clears the logs state when isReviewing transitions from false
to true, ensuring fresh logs are fetched and displayed.

* fix(security): address remaining command injection vulnerabilities

Fixes HIGH and MEDIUM severity issues from PR review:

Security fixes:
- Remove shell: true from spawn() in mcp-handlers.ts checkCommandHealth
  and testCommandConnection to prevent shell metacharacter injection
- Add command allowlist (npx, npm, node, python) and blocklist (bash,
  sh, cmd, powershell) to _validate_custom_mcp_server() in client.py
- Fix type validation mismatch: 'url' -> 'http' to match downstream usage

Quality fixes:
- Add negative value validation for UTILITY_THINKING_BUDGET in model_config.py
- Replace silent error swallowing with console.error in PRDetail.tsx

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* refactor(pr-review): extract shared utilities and reduce complexity

- Extract SDK stream processing into sdk_utils.py (~374 lines removed)
  - Callback-based architecture for message/thinking/error handling
  - Eliminates duplicate stream processing in 3+ reviewer modules

- Extract category mapping into category_utils.py (~80 lines removed)
  - Unified CATEGORY_MAPPING dictionary
  - Single source of truth for severity/category translations

- Refactor parallel_orchestrator_reviewer.py review() method
  - Split 380-line method into 165 lines + 8 focused helpers
  - Each extracted method under 50 lines for maintainability
  - Extracted: _prepare_context, _create_specialist_inputs, etc.

- Remove unused ReviewCategory imports after extraction

Total: ~454 lines of duplicate code eliminated

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(security): address all remaining PR #424 review findings

Backend security hardening (client.py):
- Reject commands with path separators (/ or \) to prevent path traversal
- Add dangerous interpreter flags blocklist (--eval, -e, -c, --exec)
- Add pwsh (PowerShell Core) to DANGEROUS_COMMANDS

Frontend defense-in-depth (mcp-handlers.ts):
- Add SAFE_COMMANDS allowlist with path validation before spawn
- Add OS-level timeout (15000ms) to testCommandConnection spawn

Model config fix:
- Treat UTILITY_THINKING_BUDGET=0 as "disable thinking" (same as empty)

SDK stream processing improvements (sdk_utils.py):
- Add try/except for stream-level and message-level errors
- Add warning when multiple StructuredOutput blocks overwrite previous
- Return error field in result dict for caller visibility

Code consolidation:
- Remove duplicate _CATEGORY_MAPPING from review_tools.py, use shared module
- Remove unreachable 'best-practices' entry in category_utils.py

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(pr-review): capture full summary content in synthesis logs

Extended the log parsing patterns to capture markdown content that
appears in the review summary. Previously, only header lines like
"Summary:" were captured, but the actual content (markdown headers,
bullet points, numbered lists, findings, file references) was
discarded because it didn't match any pattern.

Added patterns for:
- Markdown headers (##, ###)
- Bullet points and indented findings
- Bold text lines
- Numbered lists
- File references
- Additional summary fields (Is Follow-up, Resolved, etc.)

* fix(pr-review): sync PR list status with detail view using overallStatus

The PR list was computing status based only on HIGH/CRITICAL severity
findings, while the PR detail used the overallStatus field from the
backend. This caused inconsistent display where list showed "Ready to
Merge" but detail showed "Changes Requested" for MEDIUM severity issues.

Fixed by using overallStatus as the source of truth in both:
- PRList.tsx: hasBlockingFindings prop computation
- usePRFiltering.ts: getPRComputedStatus function

* fix(security): address follow-up review findings round 2

Backend security (client.py):
- Expand DANGEROUS_FLAGS to include: -m (Python module), -p (Python eval+print),
  --print, --input-type=module, --experimental-loader, --require, -r

Frontend defense-in-depth (mcp-handlers.ts):
- Add DANGEROUS_FLAGS set mirroring backend
- Add areArgsSafe() function to validate args
- Check args in both checkCommandHealth and testCommandConnection before spawn

SDK stream error handling:
- Add error field check in parallel_orchestrator_reviewer.py
- Add error field check in parallel_followup_reviewer.py
- Raise RuntimeError on stream failure instead of silently continuing

Model config:
- Add debug log when UTILITY_THINKING_BUDGET=0 disables thinking

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
2025-12-31 19:42:32 +01:00

416 lines
14 KiB
Python

"""
Tests for Bot Detection Module
================================
Tests the BotDetector class to ensure it correctly prevents infinite loops.
"""
import json
import sys
from datetime import datetime, timedelta
from pathlib import Path
from unittest.mock import MagicMock, patch
import pytest
# Add the backend runners/github directory to path
_backend_dir = Path(__file__).parent.parent / "apps" / "backend"
_github_dir = _backend_dir / "runners" / "github"
if str(_github_dir) not in sys.path:
sys.path.insert(0, str(_github_dir))
from bot_detection import BotDetectionState, BotDetector
@pytest.fixture
def temp_state_dir(tmp_path):
"""Create temporary state directory."""
state_dir = tmp_path / "github"
state_dir.mkdir()
return state_dir
@pytest.fixture
def mock_bot_detector(temp_state_dir):
"""Create bot detector with mocked bot username."""
with patch.object(BotDetector, "_get_bot_username", return_value="test-bot"):
detector = BotDetector(
state_dir=temp_state_dir,
bot_token="fake-token",
review_own_prs=False,
)
return detector
class TestBotDetectionState:
"""Test BotDetectionState data class."""
def test_save_and_load(self, temp_state_dir):
"""Test saving and loading state."""
state = BotDetectionState(
reviewed_commits={
"123": ["abc123", "def456"],
"456": ["ghi789"],
},
last_review_times={
"123": "2025-01-01T10:00:00",
"456": "2025-01-01T11:00:00",
},
)
# Save
state.save(temp_state_dir)
# Load
loaded = BotDetectionState.load(temp_state_dir)
assert loaded.reviewed_commits == state.reviewed_commits
assert loaded.last_review_times == state.last_review_times
def test_load_nonexistent(self, temp_state_dir):
"""Test loading when file doesn't exist."""
loaded = BotDetectionState.load(temp_state_dir)
assert loaded.reviewed_commits == {}
assert loaded.last_review_times == {}
class TestBotDetectorInit:
"""Test BotDetector initialization."""
def test_init_with_token(self, temp_state_dir):
"""Test initialization with bot token."""
with patch("subprocess.run") as mock_run:
mock_run.return_value = MagicMock(
returncode=0,
stdout=json.dumps({"login": "my-bot"}),
)
detector = BotDetector(
state_dir=temp_state_dir,
bot_token="ghp_test123",
review_own_prs=False,
)
assert detector.bot_username == "my-bot"
assert detector.review_own_prs is False
def test_init_without_token(self, temp_state_dir):
"""Test initialization without bot token."""
detector = BotDetector(
state_dir=temp_state_dir,
bot_token=None,
review_own_prs=True,
)
assert detector.bot_username is None
assert detector.review_own_prs is True
class TestBotDetection:
"""Test bot detection methods."""
def test_is_bot_pr(self, mock_bot_detector):
"""Test detecting bot-authored PRs."""
bot_pr = {"author": {"login": "test-bot"}}
human_pr = {"author": {"login": "alice"}}
assert mock_bot_detector.is_bot_pr(bot_pr) is True
assert mock_bot_detector.is_bot_pr(human_pr) is False
def test_is_bot_commit(self, mock_bot_detector):
"""Test detecting bot-authored commits."""
bot_commit = {"author": {"login": "test-bot"}}
human_commit = {"author": {"login": "alice"}}
bot_committer = {
"committer": {"login": "test-bot"},
"author": {"login": "alice"},
}
assert mock_bot_detector.is_bot_commit(bot_commit) is True
assert mock_bot_detector.is_bot_commit(human_commit) is False
assert mock_bot_detector.is_bot_commit(bot_committer) is True
def test_get_last_commit_sha(self, mock_bot_detector):
"""Test extracting last commit SHA."""
# GitHub API returns commits in chronological order (oldest first, newest last)
# So commits[-1] is the LATEST commit
commits = [
{"oid": "abc123"}, # Oldest commit
{"oid": "def456"}, # Latest commit
]
sha = mock_bot_detector.get_last_commit_sha(commits)
assert sha == "def456" # Should return the LAST (latest) commit
# Test with sha field instead of oid
commits_with_sha = [{"sha": "xyz789"}]
sha = mock_bot_detector.get_last_commit_sha(commits_with_sha)
assert sha == "xyz789"
# Empty commits
assert mock_bot_detector.get_last_commit_sha([]) is None
class TestCoolingOff:
"""Test cooling off period.
Note: COOLING_OFF_MINUTES is currently set to 1 minute for testing large PRs.
"""
def test_within_cooling_off(self, mock_bot_detector):
"""Test PR within cooling off period."""
# Set last review to 30 seconds ago (within 1 minute cooling off)
half_min_ago = datetime.now() - timedelta(seconds=30)
mock_bot_detector.state.last_review_times["123"] = half_min_ago.isoformat()
is_cooling, reason = mock_bot_detector.is_within_cooling_off(123)
assert is_cooling is True
assert "Cooling off" in reason
def test_outside_cooling_off(self, mock_bot_detector):
"""Test PR outside cooling off period."""
# Set last review to 2 minutes ago (outside 1 minute cooling off)
two_min_ago = datetime.now() - timedelta(minutes=2)
mock_bot_detector.state.last_review_times["123"] = two_min_ago.isoformat()
is_cooling, reason = mock_bot_detector.is_within_cooling_off(123)
assert is_cooling is False
assert reason == ""
def test_no_previous_review(self, mock_bot_detector):
"""Test PR with no previous review."""
is_cooling, reason = mock_bot_detector.is_within_cooling_off(999)
assert is_cooling is False
assert reason == ""
class TestReviewedCommits:
"""Test reviewed commit tracking."""
def test_has_reviewed_commit(self, mock_bot_detector):
"""Test checking if commit was reviewed."""
mock_bot_detector.state.reviewed_commits["123"] = ["abc123", "def456"]
assert mock_bot_detector.has_reviewed_commit(123, "abc123") is True
assert mock_bot_detector.has_reviewed_commit(123, "xyz789") is False
assert mock_bot_detector.has_reviewed_commit(999, "abc123") is False
def test_mark_reviewed(self, mock_bot_detector, temp_state_dir):
"""Test marking PR as reviewed."""
mock_bot_detector.mark_reviewed(123, "abc123")
# Check state
assert "123" in mock_bot_detector.state.reviewed_commits
assert "abc123" in mock_bot_detector.state.reviewed_commits["123"]
assert "123" in mock_bot_detector.state.last_review_times
# Check persistence
loaded = BotDetectionState.load(temp_state_dir)
assert "123" in loaded.reviewed_commits
assert "abc123" in loaded.reviewed_commits["123"]
def test_mark_reviewed_multiple(self, mock_bot_detector):
"""Test marking same PR reviewed multiple times."""
mock_bot_detector.mark_reviewed(123, "abc123")
mock_bot_detector.mark_reviewed(123, "def456")
commits = mock_bot_detector.state.reviewed_commits["123"]
assert len(commits) == 2
assert "abc123" in commits
assert "def456" in commits
class TestShouldSkipReview:
"""Test main should_skip_pr_review logic."""
def test_skip_bot_pr(self, mock_bot_detector):
"""Test skipping bot-authored PR."""
pr_data = {"author": {"login": "test-bot"}}
commits = [{"author": {"login": "test-bot"}, "oid": "abc123"}]
should_skip, reason = mock_bot_detector.should_skip_pr_review(
pr_number=123,
pr_data=pr_data,
commits=commits,
)
assert should_skip is True
assert "bot user" in reason
def test_skip_bot_commit(self, mock_bot_detector):
"""Test skipping PR with bot commit as the latest commit."""
pr_data = {"author": {"login": "alice"}}
# GitHub API returns commits in chronological order (oldest first, newest last)
# So commits[-1] is the LATEST commit - which is the bot commit
commits = [
{"author": {"login": "alice"}, "oid": "abc123"}, # Oldest commit (by alice)
{"author": {"login": "test-bot"}, "oid": "def456"}, # Latest commit (by bot)
]
should_skip, reason = mock_bot_detector.should_skip_pr_review(
pr_number=123,
pr_data=pr_data,
commits=commits,
)
assert should_skip is True
assert "bot" in reason.lower()
def test_skip_cooling_off(self, mock_bot_detector):
"""Test skipping during cooling off period."""
# Set last review to 30 seconds ago (within 1 minute cooling off)
half_min_ago = datetime.now() - timedelta(seconds=30)
mock_bot_detector.state.last_review_times["123"] = half_min_ago.isoformat()
pr_data = {"author": {"login": "alice"}}
commits = [{"author": {"login": "alice"}, "oid": "abc123"}]
should_skip, reason = mock_bot_detector.should_skip_pr_review(
pr_number=123,
pr_data=pr_data,
commits=commits,
)
assert should_skip is True
assert "Cooling off" in reason
def test_skip_already_reviewed(self, mock_bot_detector):
"""Test skipping already-reviewed commit."""
mock_bot_detector.state.reviewed_commits["123"] = ["abc123"]
pr_data = {"author": {"login": "alice"}}
commits = [{"author": {"login": "alice"}, "oid": "abc123"}]
should_skip, reason = mock_bot_detector.should_skip_pr_review(
pr_number=123,
pr_data=pr_data,
commits=commits,
)
assert should_skip is True
assert "Already reviewed" in reason
def test_allow_review(self, mock_bot_detector):
"""Test allowing review when all checks pass."""
pr_data = {"author": {"login": "alice"}}
commits = [{"author": {"login": "alice"}, "oid": "abc123"}]
should_skip, reason = mock_bot_detector.should_skip_pr_review(
pr_number=123,
pr_data=pr_data,
commits=commits,
)
assert should_skip is False
assert reason == ""
def test_allow_review_own_prs(self, temp_state_dir):
"""Test allowing review when review_own_prs is True."""
with patch.object(BotDetector, "_get_bot_username", return_value="test-bot"):
detector = BotDetector(
state_dir=temp_state_dir,
bot_token="fake-token",
review_own_prs=True, # Allow bot to review own PRs
)
pr_data = {"author": {"login": "test-bot"}}
commits = [{"author": {"login": "test-bot"}, "oid": "abc123"}]
should_skip, reason = detector.should_skip_pr_review(
pr_number=123,
pr_data=pr_data,
commits=commits,
)
# Should not skip even though it's bot's own PR
assert should_skip is False
class TestStateManagement:
"""Test state management methods."""
def test_clear_pr_state(self, mock_bot_detector, temp_state_dir):
"""Test clearing PR state."""
# Set up state
mock_bot_detector.mark_reviewed(123, "abc123")
mock_bot_detector.mark_reviewed(456, "def456")
# Clear one PR
mock_bot_detector.clear_pr_state(123)
# Check in-memory state
assert "123" not in mock_bot_detector.state.reviewed_commits
assert "123" not in mock_bot_detector.state.last_review_times
assert "456" in mock_bot_detector.state.reviewed_commits
# Check persistence
loaded = BotDetectionState.load(temp_state_dir)
assert "123" not in loaded.reviewed_commits
assert "456" in loaded.reviewed_commits
def test_get_stats(self, mock_bot_detector):
"""Test getting detector statistics."""
mock_bot_detector.mark_reviewed(123, "abc123")
mock_bot_detector.mark_reviewed(123, "def456")
mock_bot_detector.mark_reviewed(456, "ghi789")
stats = mock_bot_detector.get_stats()
assert stats["bot_username"] == "test-bot"
assert stats["review_own_prs"] is False
assert stats["total_prs_tracked"] == 2
assert stats["total_reviews_performed"] == 3
assert stats["cooling_off_minutes"] == 1 # Currently set to 1 for testing
class TestEdgeCases:
"""Test edge cases and error handling."""
def test_no_commits(self, mock_bot_detector):
"""Test handling PR with no commits."""
pr_data = {"author": {"login": "alice"}}
commits = []
should_skip, reason = mock_bot_detector.should_skip_pr_review(
pr_number=123,
pr_data=pr_data,
commits=commits,
)
# Should not skip (no bot commit to detect)
assert should_skip is False
def test_malformed_commit_data(self, mock_bot_detector):
"""Test handling malformed commit data."""
pr_data = {"author": {"login": "alice"}}
commits = [
{"author": {"login": "alice"}}, # Missing oid/sha
{}, # Empty commit
]
# Should not crash
should_skip, reason = mock_bot_detector.should_skip_pr_review(
pr_number=123,
pr_data=pr_data,
commits=commits,
)
assert should_skip is False
def test_invalid_last_review_time(self, mock_bot_detector):
"""Test handling invalid timestamp in state."""
mock_bot_detector.state.last_review_times["123"] = "invalid-timestamp"
is_cooling, reason = mock_bot_detector.is_within_cooling_off(123)
# Should not crash, should return False
assert is_cooling is False
if __name__ == "__main__":
pytest.main([__file__, "-v"])