From b0d448f0b3e65fffc50a5eb2050d8880d0401725 Mon Sep 17 00:00:00 2001 From: cabana8471 Date: Mon, 26 Jan 2026 23:04:56 +0100 Subject: [PATCH 01/11] feat: add Quality Gates to enforce lint/type-check before marking features Implements proposal from issue #96 - Quality Gates system that: - Auto-detects linters: ESLint, Biome (JS/TS), ruff, flake8 (Python) - Auto-detects type checkers: TypeScript (tsc), Python (mypy) - Supports custom quality scripts via .autocoder/quality-checks.sh - Runs quality checks before allowing feature_mark_passing - In strict mode (default), blocks marking if checks fail - Stores quality results for evidence New files: - quality_gates.py: Core quality checking module Modified files: - mcp_server/feature_mcp.py: Added feature_verify_quality tool, modified feature_mark_passing to enforce quality gates - progress.py: Added clear_stuck_features() for auto-recovery Configuration (.autocoder/config.json): ```json { "quality_gates": { "enabled": true, "strict_mode": true, "checks": { "lint": true, "type_check": true, "custom_script": null } } } ``` Addresses #96 Co-Authored-By: Claude Opus 4.5 --- mcp_server/feature_mcp.py | 72 +++++++- progress.py | 42 +++++ quality_gates.py | 376 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 488 insertions(+), 2 deletions(-) create mode 100644 quality_gates.py diff --git a/mcp_server/feature_mcp.py b/mcp_server/feature_mcp.py index a394f1e9..a18a19c3 100755 --- a/mcp_server/feature_mcp.py +++ b/mcp_server/feature_mcp.py @@ -48,6 +48,7 @@ would_create_circular_dependency, ) from api.migration import migrate_json_to_sqlite +from quality_gates import load_quality_config, verify_quality # Configuration from environment PROJECT_DIR = Path(os.environ.get("PROJECT_DIR", ".")).resolve() @@ -226,6 +227,42 @@ def feature_get_summary( session.close() +@mcp.tool() +def feature_verify_quality() -> str: + """Run quality checks (lint, type-check) on the project. + + Automatically detects and runs available linters and type checkers: + - Linters: ESLint, Biome (JS/TS), ruff, flake8 (Python) + - Type checkers: TypeScript (tsc), Python (mypy) + - Custom scripts: .autocoder/quality-checks.sh + + Use this tool before marking a feature as passing to ensure code quality. + In strict mode (default), feature_mark_passing will block if quality checks fail. + + Returns: + JSON with: passed (bool), checks (dict), summary (str) + """ + config = load_quality_config(PROJECT_DIR) + + if not config.get("enabled", True): + return json.dumps({ + "passed": True, + "checks": {}, + "summary": "Quality gates disabled" + }) + + checks_config = config.get("checks", {}) + result = verify_quality( + PROJECT_DIR, + run_lint=checks_config.get("lint", True), + run_type_check=checks_config.get("type_check", True), + run_custom=True, + custom_script_path=checks_config.get("custom_script"), + ) + + return json.dumps(result) + + @mcp.tool() def feature_mark_passing( feature_id: Annotated[int, Field(description="The ID of the feature to mark as passing", ge=1)] @@ -235,11 +272,16 @@ def feature_mark_passing( Updates the feature's passes field to true and clears the in_progress flag. Use this after you have implemented the feature and verified it works correctly. + IMPORTANT: In strict mode (default), this tool will run quality checks + (lint, type-check) and BLOCK if they fail. Run feature_verify_quality first + to see what checks will be performed. + Args: feature_id: The ID of the feature to mark as passing Returns: - JSON with success confirmation: {success, feature_id, name} + JSON with success confirmation: {success, feature_id, name, quality_result} + If strict mode is enabled and quality checks fail, returns an error. """ session = get_session() try: @@ -248,11 +290,37 @@ def feature_mark_passing( if feature is None: return json.dumps({"error": f"Feature with ID {feature_id} not found"}) + # Run quality checks + config = load_quality_config(PROJECT_DIR) + quality_result = None + + if config.get("enabled", True): + checks_config = config.get("checks", {}) + quality_result = verify_quality( + PROJECT_DIR, + run_lint=checks_config.get("lint", True), + run_type_check=checks_config.get("type_check", True), + run_custom=True, + custom_script_path=checks_config.get("custom_script"), + ) + + # In strict mode, block if quality checks failed + if config.get("strict_mode", True) and not quality_result["passed"]: + return json.dumps({ + "error": "Quality checks failed - cannot mark feature as passing", + "quality_result": quality_result, + "hint": "Fix the issues and try again, or disable strict_mode in .autocoder/config.json" + }) + feature.passes = True feature.in_progress = False session.commit() - return json.dumps({"success": True, "feature_id": feature_id, "name": feature.name}) + result = {"success": True, "feature_id": feature_id, "name": feature.name} + if quality_result: + result["quality_result"] = quality_result + + return json.dumps(result) except Exception as e: session.rollback() return json.dumps({"error": f"Failed to mark feature passing: {str(e)}"}) diff --git a/progress.py b/progress.py index 0821c90a..1298d0a1 100644 --- a/progress.py +++ b/progress.py @@ -214,6 +214,48 @@ def send_progress_webhook(passing: int, total: int, project_dir: Path) -> None: ) +def clear_stuck_features(project_dir: Path) -> int: + """ + Clear all in_progress flags from features at agent startup. + + When an agent is stopped mid-work (e.g., user interrupt, crash), + features can be left with in_progress=True and become orphaned. + This function clears those flags so features return to the pending queue. + + Args: + project_dir: Directory containing the project + + Returns: + Number of features that were unstuck + """ + db_file = project_dir / "features.db" + if not db_file.exists(): + return 0 + + try: + conn = sqlite3.connect(db_file) + cursor = conn.cursor() + + # Count how many will be cleared + cursor.execute("SELECT COUNT(*) FROM features WHERE in_progress = 1") + count = cursor.fetchone()[0] + + if count > 0: + # Clear all in_progress flags + cursor.execute("UPDATE features SET in_progress = 0 WHERE in_progress = 1") + conn.commit() + print(f"[Auto-recovery] Cleared {count} stuck feature(s) from previous session") + + conn.close() + return count + except sqlite3.OperationalError: + # Table doesn't exist or doesn't have in_progress column + return 0 + except Exception as e: + print(f"[Warning] Could not clear stuck features: {e}") + return 0 + + def print_session_header(session_num: int, is_initializer: bool) -> None: """Print a formatted header for the session.""" session_type = "INITIALIZER" if is_initializer else "CODING AGENT" diff --git a/quality_gates.py b/quality_gates.py new file mode 100644 index 00000000..927fc716 --- /dev/null +++ b/quality_gates.py @@ -0,0 +1,376 @@ +""" +Quality Gates Module +==================== + +Provides quality checking functionality for the Autocoder system. +Runs lint, type-check, and custom scripts before allowing features +to be marked as passing. + +Supports: +- ESLint/Biome for JavaScript/TypeScript +- ruff/flake8 for Python +- Custom scripts via .autocoder/quality-checks.sh +""" + +import json +import shutil +import subprocess +from datetime import datetime +from pathlib import Path +from typing import TypedDict + + +class QualityCheckResult(TypedDict): + """Result of a single quality check.""" + name: str + passed: bool + output: str + duration_ms: int + + +class QualityGateResult(TypedDict): + """Result of all quality checks combined.""" + passed: bool + timestamp: str + checks: dict[str, QualityCheckResult] + summary: str + + +def _run_command(cmd: list[str], cwd: Path, timeout: int = 60) -> tuple[int, str, int]: + """ + Run a command and return (exit_code, output, duration_ms). + + Args: + cmd: Command and arguments as a list + cwd: Working directory + timeout: Timeout in seconds + + Returns: + (exit_code, combined_output, duration_ms) + """ + import time + start = time.time() + + try: + result = subprocess.run( + cmd, + cwd=cwd, + capture_output=True, + text=True, + timeout=timeout, + ) + duration_ms = int((time.time() - start) * 1000) + output = result.stdout + result.stderr + return result.returncode, output.strip(), duration_ms + except subprocess.TimeoutExpired: + duration_ms = int((time.time() - start) * 1000) + return 124, f"Command timed out after {timeout}s", duration_ms + except FileNotFoundError: + return 127, f"Command not found: {cmd[0]}", 0 + except Exception as e: + return 1, str(e), 0 + + +def _detect_js_linter(project_dir: Path) -> tuple[str, list[str]] | None: + """ + Detect the JavaScript/TypeScript linter to use. + + Returns: + (name, command) tuple, or None if no linter detected + """ + # Check for ESLint + if (project_dir / "node_modules/.bin/eslint").exists(): + return ("eslint", ["node_modules/.bin/eslint", ".", "--max-warnings=0"]) + + # Check for Biome + if (project_dir / "node_modules/.bin/biome").exists(): + return ("biome", ["node_modules/.bin/biome", "lint", "."]) + + # Check for package.json lint script + package_json = project_dir / "package.json" + if package_json.exists(): + try: + data = json.loads(package_json.read_text()) + scripts = data.get("scripts", {}) + if "lint" in scripts: + return ("npm_lint", ["npm", "run", "lint"]) + except (json.JSONDecodeError, OSError): + pass + + return None + + +def _detect_python_linter(project_dir: Path) -> tuple[str, list[str]] | None: + """ + Detect the Python linter to use. + + Returns: + (name, command) tuple, or None if no linter detected + """ + # Check for ruff + if shutil.which("ruff"): + return ("ruff", ["ruff", "check", "."]) + + # Check for flake8 + if shutil.which("flake8"): + return ("flake8", ["flake8", "."]) + + # Check in virtual environment + venv_ruff = project_dir / "venv/bin/ruff" + if venv_ruff.exists(): + return ("ruff", [str(venv_ruff), "check", "."]) + + venv_flake8 = project_dir / "venv/bin/flake8" + if venv_flake8.exists(): + return ("flake8", [str(venv_flake8), "."]) + + return None + + +def _detect_type_checker(project_dir: Path) -> tuple[str, list[str]] | None: + """ + Detect the type checker to use. + + Returns: + (name, command) tuple, or None if no type checker detected + """ + # TypeScript + if (project_dir / "tsconfig.json").exists(): + if (project_dir / "node_modules/.bin/tsc").exists(): + return ("tsc", ["node_modules/.bin/tsc", "--noEmit"]) + if shutil.which("npx"): + return ("tsc", ["npx", "tsc", "--noEmit"]) + + # Python (mypy) + if (project_dir / "pyproject.toml").exists() or (project_dir / "setup.py").exists(): + if shutil.which("mypy"): + return ("mypy", ["mypy", "."]) + venv_mypy = project_dir / "venv/bin/mypy" + if venv_mypy.exists(): + return ("mypy", [str(venv_mypy), "."]) + + return None + + +def run_lint_check(project_dir: Path) -> QualityCheckResult: + """ + Run lint check on the project. + + Automatically detects the appropriate linter based on project type. + + Args: + project_dir: Path to the project directory + + Returns: + QualityCheckResult with lint results + """ + # Try JS/TS linter first + linter = _detect_js_linter(project_dir) + if linter is None: + # Try Python linter + linter = _detect_python_linter(project_dir) + + if linter is None: + return { + "name": "lint", + "passed": True, + "output": "No linter detected, skipping lint check", + "duration_ms": 0, + } + + name, cmd = linter + exit_code, output, duration_ms = _run_command(cmd, project_dir) + + # Truncate output if too long + if len(output) > 5000: + output = output[:5000] + "\n... (truncated)" + + return { + "name": f"lint ({name})", + "passed": exit_code == 0, + "output": output if output else "No issues found", + "duration_ms": duration_ms, + } + + +def run_type_check(project_dir: Path) -> QualityCheckResult: + """ + Run type check on the project. + + Automatically detects the appropriate type checker based on project type. + + Args: + project_dir: Path to the project directory + + Returns: + QualityCheckResult with type check results + """ + checker = _detect_type_checker(project_dir) + + if checker is None: + return { + "name": "type_check", + "passed": True, + "output": "No type checker detected, skipping type check", + "duration_ms": 0, + } + + name, cmd = checker + exit_code, output, duration_ms = _run_command(cmd, project_dir, timeout=120) + + # Truncate output if too long + if len(output) > 5000: + output = output[:5000] + "\n... (truncated)" + + return { + "name": f"type_check ({name})", + "passed": exit_code == 0, + "output": output if output else "No type errors found", + "duration_ms": duration_ms, + } + + +def run_custom_script(project_dir: Path, script_path: str | None = None) -> QualityCheckResult | None: + """ + Run a custom quality check script. + + Args: + project_dir: Path to the project directory + script_path: Path to the script (relative to project), defaults to .autocoder/quality-checks.sh + + Returns: + QualityCheckResult, or None if script doesn't exist + """ + if script_path is None: + script_path = ".autocoder/quality-checks.sh" + + script_full_path = project_dir / script_path + + if not script_full_path.exists(): + return None + + # Make sure it's executable + try: + script_full_path.chmod(0o755) + except OSError: + pass + + exit_code, output, duration_ms = _run_command( + ["bash", str(script_full_path)], + project_dir, + timeout=300, # 5 minutes for custom scripts + ) + + # Truncate output if too long + if len(output) > 10000: + output = output[:10000] + "\n... (truncated)" + + return { + "name": "custom_script", + "passed": exit_code == 0, + "output": output if output else "Script completed successfully", + "duration_ms": duration_ms, + } + + +def verify_quality( + project_dir: Path, + run_lint: bool = True, + run_type_check: bool = True, + run_custom: bool = True, + custom_script_path: str | None = None, +) -> QualityGateResult: + """ + Run all configured quality checks. + + Args: + project_dir: Path to the project directory + run_lint: Whether to run lint check + run_type_check: Whether to run type check + run_custom: Whether to run custom script + custom_script_path: Path to custom script (optional) + + Returns: + QualityGateResult with all check results + """ + checks: dict[str, QualityCheckResult] = {} + all_passed = True + + if run_lint: + lint_result = run_lint_check(project_dir) + checks["lint"] = lint_result + if not lint_result["passed"]: + all_passed = False + + if run_type_check: + type_result = run_type_check(project_dir) + checks["type_check"] = type_result + if not type_result["passed"]: + all_passed = False + + if run_custom: + custom_result = run_custom_script(project_dir, custom_script_path) + if custom_result is not None: + checks["custom_script"] = custom_result + if not custom_result["passed"]: + all_passed = False + + # Build summary + passed_count = sum(1 for c in checks.values() if c["passed"]) + total_count = len(checks) + failed_names = [name for name, c in checks.items() if not c["passed"]] + + if all_passed: + summary = f"All {total_count} quality checks passed" + else: + summary = f"{passed_count}/{total_count} checks passed. Failed: {', '.join(failed_names)}" + + return { + "passed": all_passed, + "timestamp": datetime.utcnow().isoformat(), + "checks": checks, + "summary": summary, + } + + +def load_quality_config(project_dir: Path) -> dict: + """ + Load quality gates configuration from .autocoder/config.json. + + Args: + project_dir: Path to the project directory + + Returns: + Quality gates config dict with defaults applied + """ + defaults = { + "enabled": True, + "strict_mode": True, + "checks": { + "lint": True, + "type_check": True, + "unit_tests": False, + "custom_script": None, + }, + } + + config_path = project_dir / ".autocoder" / "config.json" + if not config_path.exists(): + return defaults + + try: + data = json.loads(config_path.read_text()) + quality_config = data.get("quality_gates", {}) + + # Merge with defaults + result = defaults.copy() + for key in ["enabled", "strict_mode"]: + if key in quality_config: + result[key] = quality_config[key] + + if "checks" in quality_config: + result["checks"] = {**defaults["checks"], **quality_config["checks"]} + + return result + except (json.JSONDecodeError, OSError): + return defaults From 45ceb5b71b1da8f4ff41275d06f65c5d90b852a5 Mon Sep 17 00:00:00 2001 From: cabana8471 Date: Tue, 27 Jan 2026 06:58:46 +0100 Subject: [PATCH 02/11] fix: address CodeRabbit review feedback - Move quality checks before DB session to avoid holding locks - Return error instead of None for missing configured custom script - Use contextlib.closing for SQLite connections in progress.py Co-Authored-By: Claude Opus 4.5 --- mcp_server/feature_mcp.py | 451 +++++++++++++++++++++----------------- progress.py | 142 ++++++------ quality_gates.py | 26 ++- 3 files changed, 356 insertions(+), 263 deletions(-) diff --git a/mcp_server/feature_mcp.py b/mcp_server/feature_mcp.py index a18a19c3..fba4c4ba 100755 --- a/mcp_server/feature_mcp.py +++ b/mcp_server/feature_mcp.py @@ -30,18 +30,18 @@ import json import os import sys -import threading from contextlib import asynccontextmanager from pathlib import Path from typing import Annotated from mcp.server.fastmcp import FastMCP from pydantic import BaseModel, Field +from sqlalchemy import text # Add parent directory to path so we can import from api module sys.path.insert(0, str(Path(__file__).parent.parent)) -from api.database import Feature, create_database +from api.database import atomic_transaction, create_database, Feature from api.dependency_resolver import ( MAX_DEPENDENCIES_PER_FEATURE, compute_scheduling_scores, @@ -97,8 +97,9 @@ class BulkCreateInput(BaseModel): _session_maker = None _engine = None -# Lock for priority assignment to prevent race conditions -_priority_lock = threading.Lock() +# NOTE: The old threading.Lock() was removed because it only worked per-process, +# not cross-process. In parallel mode, multiple MCP servers run in separate +# processes, so the lock was useless. We now use atomic SQL operations instead. @asynccontextmanager @@ -272,6 +273,8 @@ def feature_mark_passing( Updates the feature's passes field to true and clears the in_progress flag. Use this after you have implemented the feature and verified it works correctly. + Uses atomic SQL UPDATE for parallel safety. + IMPORTANT: In strict mode (default), this tool will run quality checks (lint, type-check) and BLOCK if they fail. Run feature_verify_quality first to see what checks will be performed. @@ -283,40 +286,47 @@ def feature_mark_passing( JSON with success confirmation: {success, feature_id, name, quality_result} If strict mode is enabled and quality checks fail, returns an error. """ + # Run quality checks BEFORE opening DB session to avoid holding locks + config = load_quality_config(PROJECT_DIR) + quality_result = None + + if config.get("enabled", True): + checks_config = config.get("checks", {}) + quality_result = verify_quality( + PROJECT_DIR, + run_lint=checks_config.get("lint", True), + run_type_check=checks_config.get("type_check", True), + run_custom=True, + custom_script_path=checks_config.get("custom_script"), + ) + + # In strict mode, block if quality checks failed + if config.get("strict_mode", True) and not quality_result["passed"]: + return json.dumps({ + "error": "Quality checks failed - cannot mark feature as passing", + "quality_result": quality_result, + "hint": "Fix the issues and try again, or disable strict_mode in .autocoder/config.json" + }) + + # Now open DB session for the atomic update session = get_session() try: + # First get the feature name for the response feature = session.query(Feature).filter(Feature.id == feature_id).first() - if feature is None: return json.dumps({"error": f"Feature with ID {feature_id} not found"}) - # Run quality checks - config = load_quality_config(PROJECT_DIR) - quality_result = None - - if config.get("enabled", True): - checks_config = config.get("checks", {}) - quality_result = verify_quality( - PROJECT_DIR, - run_lint=checks_config.get("lint", True), - run_type_check=checks_config.get("type_check", True), - run_custom=True, - custom_script_path=checks_config.get("custom_script"), - ) + name = feature.name - # In strict mode, block if quality checks failed - if config.get("strict_mode", True) and not quality_result["passed"]: - return json.dumps({ - "error": "Quality checks failed - cannot mark feature as passing", - "quality_result": quality_result, - "hint": "Fix the issues and try again, or disable strict_mode in .autocoder/config.json" - }) - - feature.passes = True - feature.in_progress = False + # Atomic update - prevents race conditions in parallel mode + session.execute(text(""" + UPDATE features + SET passes = 1, in_progress = 0 + WHERE id = :id + """), {"id": feature_id}) session.commit() - result = {"success": True, "feature_id": feature_id, "name": feature.name} + result = {"success": True, "feature_id": feature_id, "name": name} if quality_result: result["quality_result"] = quality_result @@ -338,6 +348,8 @@ def feature_mark_failing( Use this when a testing agent discovers that a previously-passing feature no longer works correctly (regression detected). + Uses atomic SQL UPDATE for parallel safety. + After marking as failing, you should: 1. Investigate the root cause 2. Fix the regression @@ -352,14 +364,20 @@ def feature_mark_failing( """ session = get_session() try: + # Check if feature exists first feature = session.query(Feature).filter(Feature.id == feature_id).first() - if feature is None: return json.dumps({"error": f"Feature with ID {feature_id} not found"}) - feature.passes = False - feature.in_progress = False + # Atomic update - prevents race conditions in parallel mode + session.execute(text(""" + UPDATE features + SET passes = 0, in_progress = 0 + WHERE id = :id + """), {"id": feature_id}) session.commit() + + # Refresh to get updated state session.refresh(feature) return json.dumps({ @@ -388,6 +406,8 @@ def feature_skip( worked on after all other pending features. Also clears the in_progress flag so the feature returns to "pending" status. + Uses atomic SQL UPDATE with subquery for parallel safety. + Args: feature_id: The ID of the feature to skip @@ -405,25 +425,28 @@ def feature_skip( return json.dumps({"error": "Cannot skip a feature that is already passing"}) old_priority = feature.priority + name = feature.name + + # Atomic update: set priority to max+1 in a single statement + # This prevents race conditions where two features get the same priority + session.execute(text(""" + UPDATE features + SET priority = (SELECT COALESCE(MAX(priority), 0) + 1 FROM features), + in_progress = 0 + WHERE id = :id + """), {"id": feature_id}) + session.commit() - # Use lock to prevent race condition in priority assignment - with _priority_lock: - # Get max priority and set this feature to max + 1 - max_priority_result = session.query(Feature.priority).order_by(Feature.priority.desc()).first() - new_priority = (max_priority_result[0] + 1) if max_priority_result else 1 - - feature.priority = new_priority - feature.in_progress = False - session.commit() - + # Refresh to get new priority session.refresh(feature) + new_priority = feature.priority return json.dumps({ - "id": feature.id, - "name": feature.name, + "id": feature_id, + "name": name, "old_priority": old_priority, "new_priority": new_priority, - "message": f"Feature '{feature.name}' moved to end of queue" + "message": f"Feature '{name}' moved to end of queue" }) except Exception as e: session.rollback() @@ -441,6 +464,9 @@ def feature_mark_in_progress( This prevents other agent sessions from working on the same feature. Call this after getting your assigned feature details with feature_get_by_id. + Uses atomic UPDATE WHERE for parallel safety - prevents two agents from + claiming the same feature simultaneously. + Args: feature_id: The ID of the feature to mark as in-progress @@ -449,21 +475,27 @@ def feature_mark_in_progress( """ session = get_session() try: - feature = session.query(Feature).filter(Feature.id == feature_id).first() - - if feature is None: - return json.dumps({"error": f"Feature with ID {feature_id} not found"}) - - if feature.passes: - return json.dumps({"error": f"Feature with ID {feature_id} is already passing"}) - - if feature.in_progress: - return json.dumps({"error": f"Feature with ID {feature_id} is already in-progress"}) - - feature.in_progress = True + # Atomic claim: only succeeds if feature is not already claimed or passing + result = session.execute(text(""" + UPDATE features + SET in_progress = 1 + WHERE id = :id AND passes = 0 AND in_progress = 0 + """), {"id": feature_id}) session.commit() - session.refresh(feature) + if result.rowcount == 0: + # Check why the claim failed + feature = session.query(Feature).filter(Feature.id == feature_id).first() + if feature is None: + return json.dumps({"error": f"Feature with ID {feature_id} not found"}) + if feature.passes: + return json.dumps({"error": f"Feature with ID {feature_id} is already passing"}) + if feature.in_progress: + return json.dumps({"error": f"Feature with ID {feature_id} is already in-progress"}) + return json.dumps({"error": "Failed to mark feature in-progress for unknown reason"}) + + # Fetch the claimed feature + feature = session.query(Feature).filter(Feature.id == feature_id).first() return json.dumps(feature.to_dict()) except Exception as e: session.rollback() @@ -481,6 +513,8 @@ def feature_claim_and_get( Combines feature_mark_in_progress + feature_get_by_id into a single operation. If already in-progress, still returns the feature details (idempotent). + Uses atomic UPDATE WHERE for parallel safety. + Args: feature_id: The ID of the feature to claim and retrieve @@ -489,24 +523,35 @@ def feature_claim_and_get( """ session = get_session() try: + # First check if feature exists and get initial state feature = session.query(Feature).filter(Feature.id == feature_id).first() - if feature is None: return json.dumps({"error": f"Feature with ID {feature_id} not found"}) if feature.passes: return json.dumps({"error": f"Feature with ID {feature_id} is already passing"}) - # Idempotent: if already in-progress, just return details - already_claimed = feature.in_progress - if not already_claimed: - feature.in_progress = True - session.commit() + # Try atomic claim: only succeeds if not already claimed + result = session.execute(text(""" + UPDATE features + SET in_progress = 1 + WHERE id = :id AND passes = 0 AND in_progress = 0 + """), {"id": feature_id}) + session.commit() + + # Determine if we claimed it or it was already claimed + already_claimed = result.rowcount == 0 + if already_claimed: + # Verify it's in_progress (not some other failure condition) session.refresh(feature) + if not feature.in_progress: + return json.dumps({"error": f"Failed to claim feature {feature_id} for unknown reason"}) - result = feature.to_dict() - result["already_claimed"] = already_claimed - return json.dumps(result) + # Refresh to get current state + session.refresh(feature) + result_dict = feature.to_dict() + result_dict["already_claimed"] = already_claimed + return json.dumps(result_dict) except Exception as e: session.rollback() return json.dumps({"error": f"Failed to claim feature: {str(e)}"}) @@ -523,6 +568,8 @@ def feature_clear_in_progress( Use this when abandoning a feature or manually unsticking a stuck feature. The feature will return to the pending queue. + Uses atomic SQL UPDATE for parallel safety. + Args: feature_id: The ID of the feature to clear in-progress status @@ -531,15 +578,20 @@ def feature_clear_in_progress( """ session = get_session() try: + # Check if feature exists feature = session.query(Feature).filter(Feature.id == feature_id).first() - if feature is None: return json.dumps({"error": f"Feature with ID {feature_id} not found"}) - feature.in_progress = False + # Atomic update - idempotent, safe in parallel mode + session.execute(text(""" + UPDATE features + SET in_progress = 0 + WHERE id = :id + """), {"id": feature_id}) session.commit() - session.refresh(feature) + session.refresh(feature) return json.dumps(feature.to_dict()) except Exception as e: session.rollback() @@ -560,6 +612,8 @@ def feature_create_bulk( This is typically used by the initializer agent to set up the initial feature list from the app specification. + Uses EXCLUSIVE transaction to prevent priority collisions in parallel mode. + Args: features: List of features to create, each with: - category (str): Feature category @@ -574,13 +628,14 @@ def feature_create_bulk( Returns: JSON with: created (int) - number of features created, with_dependencies (int) """ - session = get_session() try: - # Use lock to prevent race condition in priority assignment - with _priority_lock: - # Get the starting priority - max_priority_result = session.query(Feature.priority).order_by(Feature.priority.desc()).first() - start_priority = (max_priority_result[0] + 1) if max_priority_result else 1 + # Use EXCLUSIVE transaction for bulk inserts to prevent conflicts + with atomic_transaction(_session_maker, "EXCLUSIVE") as session: + # Get the starting priority atomically within the transaction + result = session.execute(text(""" + SELECT COALESCE(MAX(priority), 0) FROM features + """)).fetchone() + start_priority = (result[0] or 0) + 1 # First pass: validate all features and their index-based dependencies for i, feature_data in enumerate(features): @@ -614,11 +669,11 @@ def feature_create_bulk( "error": f"Feature at index {i} cannot depend on feature at index {idx} (forward reference not allowed)" }) - # Second pass: create all features + # Second pass: create all features with reserved priorities created_features: list[Feature] = [] for i, feature_data in enumerate(features): db_feature = Feature( - priority=start_priority + i, + priority=start_priority + i, # Guaranteed unique within EXCLUSIVE transaction category=feature_data["category"], name=feature_data["name"], description=feature_data["description"], @@ -642,17 +697,13 @@ def feature_create_bulk( created_features[i].dependencies = sorted(dep_ids) deps_count += 1 - session.commit() - - return json.dumps({ - "created": len(created_features), - "with_dependencies": deps_count - }) + # Commit happens automatically on context manager exit + return json.dumps({ + "created": len(created_features), + "with_dependencies": deps_count + }) except Exception as e: - session.rollback() return json.dumps({"error": str(e)}) - finally: - session.close() @mcp.tool() @@ -667,6 +718,8 @@ def feature_create( Use this when the user asks to add a new feature, capability, or test case. The feature will be added with the next available priority number. + Uses IMMEDIATE transaction for parallel safety. + Args: category: Feature category for grouping (e.g., 'Authentication', 'API', 'UI') name: Descriptive name for the feature @@ -676,13 +729,14 @@ def feature_create( Returns: JSON with the created feature details including its ID """ - session = get_session() try: - # Use lock to prevent race condition in priority assignment - with _priority_lock: - # Get the next priority - max_priority_result = session.query(Feature.priority).order_by(Feature.priority.desc()).first() - next_priority = (max_priority_result[0] + 1) if max_priority_result else 1 + # Use IMMEDIATE transaction to prevent priority collisions + with atomic_transaction(_session_maker, "IMMEDIATE") as session: + # Get the next priority atomically within the transaction + result = session.execute(text(""" + SELECT COALESCE(MAX(priority), 0) + 1 FROM features + """)).fetchone() + next_priority = result[0] db_feature = Feature( priority=next_priority, @@ -694,20 +748,18 @@ def feature_create( in_progress=False, ) session.add(db_feature) - session.commit() + session.flush() # Get the ID - session.refresh(db_feature) + feature_dict = db_feature.to_dict() + # Commit happens automatically on context manager exit return json.dumps({ "success": True, "message": f"Created feature: {name}", - "feature": db_feature.to_dict() + "feature": feature_dict }) except Exception as e: - session.rollback() return json.dumps({"error": str(e)}) - finally: - session.close() @mcp.tool() @@ -720,6 +772,8 @@ def feature_add_dependency( The dependency_id feature must be completed before feature_id can be started. Validates: self-reference, existence, circular dependencies, max limit. + Uses IMMEDIATE transaction to prevent stale reads during cycle detection. + Args: feature_id: The ID of the feature that will depend on another feature dependency_id: The ID of the feature that must be completed first @@ -727,52 +781,49 @@ def feature_add_dependency( Returns: JSON with success status and updated dependencies list, or error message """ - session = get_session() try: - # Security: Self-reference check + # Security: Self-reference check (can do before transaction) if feature_id == dependency_id: return json.dumps({"error": "A feature cannot depend on itself"}) - feature = session.query(Feature).filter(Feature.id == feature_id).first() - dependency = session.query(Feature).filter(Feature.id == dependency_id).first() - - if not feature: - return json.dumps({"error": f"Feature {feature_id} not found"}) - if not dependency: - return json.dumps({"error": f"Dependency feature {dependency_id} not found"}) - - current_deps = feature.dependencies or [] - - # Security: Max dependencies limit - if len(current_deps) >= MAX_DEPENDENCIES_PER_FEATURE: - return json.dumps({"error": f"Maximum {MAX_DEPENDENCIES_PER_FEATURE} dependencies allowed per feature"}) - - # Check if already exists - if dependency_id in current_deps: - return json.dumps({"error": "Dependency already exists"}) - - # Security: Circular dependency check - # would_create_circular_dependency(features, source_id, target_id) - # source_id = feature gaining the dependency, target_id = feature being depended upon - all_features = [f.to_dict() for f in session.query(Feature).all()] - if would_create_circular_dependency(all_features, feature_id, dependency_id): - return json.dumps({"error": "Cannot add: would create circular dependency"}) - - # Add dependency - current_deps.append(dependency_id) - feature.dependencies = sorted(current_deps) - session.commit() - - return json.dumps({ - "success": True, - "feature_id": feature_id, - "dependencies": feature.dependencies - }) + # Use IMMEDIATE transaction for consistent cycle detection + with atomic_transaction(_session_maker, "IMMEDIATE") as session: + feature = session.query(Feature).filter(Feature.id == feature_id).first() + dependency = session.query(Feature).filter(Feature.id == dependency_id).first() + + if not feature: + return json.dumps({"error": f"Feature {feature_id} not found"}) + if not dependency: + return json.dumps({"error": f"Dependency feature {dependency_id} not found"}) + + current_deps = feature.dependencies or [] + + # Security: Max dependencies limit + if len(current_deps) >= MAX_DEPENDENCIES_PER_FEATURE: + return json.dumps({"error": f"Maximum {MAX_DEPENDENCIES_PER_FEATURE} dependencies allowed per feature"}) + + # Check if already exists + if dependency_id in current_deps: + return json.dumps({"error": "Dependency already exists"}) + + # Security: Circular dependency check + # Within IMMEDIATE transaction, snapshot is protected by write lock + all_features = [f.to_dict() for f in session.query(Feature).all()] + if would_create_circular_dependency(all_features, feature_id, dependency_id): + return json.dumps({"error": "Cannot add: would create circular dependency"}) + + # Add dependency atomically + new_deps = sorted(current_deps + [dependency_id]) + feature.dependencies = new_deps + # Commit happens automatically on context manager exit + + return json.dumps({ + "success": True, + "feature_id": feature_id, + "dependencies": new_deps + }) except Exception as e: - session.rollback() return json.dumps({"error": f"Failed to add dependency: {str(e)}"}) - finally: - session.close() @mcp.tool() @@ -782,6 +833,8 @@ def feature_remove_dependency( ) -> str: """Remove a dependency from a feature. + Uses IMMEDIATE transaction for parallel safety. + Args: feature_id: The ID of the feature to remove a dependency from dependency_id: The ID of the dependency to remove @@ -789,30 +842,29 @@ def feature_remove_dependency( Returns: JSON with success status and updated dependencies list, or error message """ - session = get_session() try: - feature = session.query(Feature).filter(Feature.id == feature_id).first() - if not feature: - return json.dumps({"error": f"Feature {feature_id} not found"}) - - current_deps = feature.dependencies or [] - if dependency_id not in current_deps: - return json.dumps({"error": "Dependency does not exist"}) - - current_deps.remove(dependency_id) - feature.dependencies = current_deps if current_deps else None - session.commit() - - return json.dumps({ - "success": True, - "feature_id": feature_id, - "dependencies": feature.dependencies or [] - }) + # Use IMMEDIATE transaction for consistent read-modify-write + with atomic_transaction(_session_maker, "IMMEDIATE") as session: + feature = session.query(Feature).filter(Feature.id == feature_id).first() + if not feature: + return json.dumps({"error": f"Feature {feature_id} not found"}) + + current_deps = feature.dependencies or [] + if dependency_id not in current_deps: + return json.dumps({"error": "Dependency does not exist"}) + + # Remove dependency atomically + new_deps = [d for d in current_deps if d != dependency_id] + feature.dependencies = new_deps if new_deps else None + # Commit happens automatically on context manager exit + + return json.dumps({ + "success": True, + "feature_id": feature_id, + "dependencies": new_deps + }) except Exception as e: - session.rollback() return json.dumps({"error": f"Failed to remove dependency: {str(e)}"}) - finally: - session.close() @mcp.tool() @@ -958,6 +1010,8 @@ def feature_set_dependencies( Validates: self-reference, existence of all dependencies, circular dependencies, max limit. + Uses IMMEDIATE transaction to prevent stale reads during cycle detection. + Args: feature_id: The ID of the feature to set dependencies for dependency_ids: List of feature IDs that must be completed first @@ -965,9 +1019,8 @@ def feature_set_dependencies( Returns: JSON with success status and updated dependencies list, or error message """ - session = get_session() try: - # Security: Self-reference check + # Security: Self-reference check (can do before transaction) if feature_id in dependency_ids: return json.dumps({"error": "A feature cannot depend on itself"}) @@ -979,45 +1032,45 @@ def feature_set_dependencies( if len(dependency_ids) != len(set(dependency_ids)): return json.dumps({"error": "Duplicate dependencies not allowed"}) - feature = session.query(Feature).filter(Feature.id == feature_id).first() - if not feature: - return json.dumps({"error": f"Feature {feature_id} not found"}) - - # Validate all dependencies exist - all_feature_ids = {f.id for f in session.query(Feature).all()} - missing = [d for d in dependency_ids if d not in all_feature_ids] - if missing: - return json.dumps({"error": f"Dependencies not found: {missing}"}) - - # Check for circular dependencies - all_features = [f.to_dict() for f in session.query(Feature).all()] - # Temporarily update the feature's dependencies for cycle check - test_features = [] - for f in all_features: - if f["id"] == feature_id: - test_features.append({**f, "dependencies": dependency_ids}) - else: - test_features.append(f) - - for dep_id in dependency_ids: - # source_id = feature_id (gaining dep), target_id = dep_id (being depended upon) - if would_create_circular_dependency(test_features, feature_id, dep_id): - return json.dumps({"error": f"Cannot add dependency {dep_id}: would create circular dependency"}) - - # Set dependencies - feature.dependencies = sorted(dependency_ids) if dependency_ids else None - session.commit() - - return json.dumps({ - "success": True, - "feature_id": feature_id, - "dependencies": feature.dependencies or [] - }) + # Use IMMEDIATE transaction for consistent cycle detection + with atomic_transaction(_session_maker, "IMMEDIATE") as session: + feature = session.query(Feature).filter(Feature.id == feature_id).first() + if not feature: + return json.dumps({"error": f"Feature {feature_id} not found"}) + + # Validate all dependencies exist + all_feature_ids = {f.id for f in session.query(Feature).all()} + missing = [d for d in dependency_ids if d not in all_feature_ids] + if missing: + return json.dumps({"error": f"Dependencies not found: {missing}"}) + + # Check for circular dependencies + # Within IMMEDIATE transaction, snapshot is protected by write lock + all_features = [f.to_dict() for f in session.query(Feature).all()] + # Temporarily update the feature's dependencies for cycle check + test_features = [] + for f in all_features: + if f["id"] == feature_id: + test_features.append({**f, "dependencies": dependency_ids}) + else: + test_features.append(f) + + for dep_id in dependency_ids: + if would_create_circular_dependency(test_features, feature_id, dep_id): + return json.dumps({"error": f"Cannot add dependency {dep_id}: would create circular dependency"}) + + # Set dependencies atomically + sorted_deps = sorted(dependency_ids) if dependency_ids else None + feature.dependencies = sorted_deps + # Commit happens automatically on context manager exit + + return json.dumps({ + "success": True, + "feature_id": feature_id, + "dependencies": sorted_deps or [] + }) except Exception as e: - session.rollback() return json.dumps({"error": f"Failed to set dependencies: {str(e)}"}) - finally: - session.close() if __name__ == "__main__": diff --git a/progress.py b/progress.py index 1298d0a1..85f5d61c 100644 --- a/progress.py +++ b/progress.py @@ -10,12 +10,34 @@ import os import sqlite3 import urllib.request +from contextlib import closing from datetime import datetime, timezone from pathlib import Path WEBHOOK_URL = os.environ.get("PROGRESS_N8N_WEBHOOK_URL") PROGRESS_CACHE_FILE = ".progress_cache" +# SQLite connection settings for parallel mode safety +SQLITE_TIMEOUT = 30 # seconds to wait for locks +SQLITE_BUSY_TIMEOUT_MS = 30000 # milliseconds for PRAGMA busy_timeout + + +def _get_connection(db_file: Path) -> sqlite3.Connection: + """Get a SQLite connection with proper timeout settings. + + Uses timeout=30s and PRAGMA busy_timeout=30000 for safe operation + in parallel mode where multiple processes access the same database. + + Args: + db_file: Path to the SQLite database file + + Returns: + sqlite3.Connection with proper timeout settings + """ + conn = sqlite3.connect(db_file, timeout=SQLITE_TIMEOUT) + conn.execute(f"PRAGMA busy_timeout={SQLITE_BUSY_TIMEOUT_MS}") + return conn + def has_features(project_dir: Path) -> bool: """ @@ -31,8 +53,6 @@ def has_features(project_dir: Path) -> bool: Returns False if no features exist (initializer needs to run). """ - import sqlite3 - # Check legacy JSON file first json_file = project_dir / "feature_list.json" if json_file.exists(): @@ -44,12 +64,11 @@ def has_features(project_dir: Path) -> bool: return False try: - conn = sqlite3.connect(db_file) - cursor = conn.cursor() - cursor.execute("SELECT COUNT(*) FROM features") - count = cursor.fetchone()[0] - conn.close() - return count > 0 + with closing(_get_connection(db_file)) as conn: + cursor = conn.cursor() + cursor.execute("SELECT COUNT(*) FROM features") + count = cursor.fetchone()[0] + return count > 0 except Exception: # Database exists but can't be read or has no features table return False @@ -59,6 +78,8 @@ def count_passing_tests(project_dir: Path) -> tuple[int, int, int]: """ Count passing, in_progress, and total tests via direct database access. + Uses connection with proper timeout settings for parallel mode safety. + Args: project_dir: Directory containing the project @@ -70,36 +91,35 @@ def count_passing_tests(project_dir: Path) -> tuple[int, int, int]: return 0, 0, 0 try: - conn = sqlite3.connect(db_file) - cursor = conn.cursor() - # Single aggregate query instead of 3 separate COUNT queries - # Handle case where in_progress column doesn't exist yet (legacy DBs) - try: - cursor.execute(""" - SELECT - COUNT(*) as total, - SUM(CASE WHEN passes = 1 THEN 1 ELSE 0 END) as passing, - SUM(CASE WHEN in_progress = 1 THEN 1 ELSE 0 END) as in_progress - FROM features - """) - row = cursor.fetchone() - total = row[0] or 0 - passing = row[1] or 0 - in_progress = row[2] or 0 - except sqlite3.OperationalError: - # Fallback for databases without in_progress column - cursor.execute(""" - SELECT - COUNT(*) as total, - SUM(CASE WHEN passes = 1 THEN 1 ELSE 0 END) as passing - FROM features - """) - row = cursor.fetchone() - total = row[0] or 0 - passing = row[1] or 0 - in_progress = 0 - conn.close() - return passing, in_progress, total + with closing(_get_connection(db_file)) as conn: + cursor = conn.cursor() + # Single aggregate query instead of 3 separate COUNT queries + # Handle case where in_progress column doesn't exist yet (legacy DBs) + try: + cursor.execute(""" + SELECT + COUNT(*) as total, + SUM(CASE WHEN passes = 1 THEN 1 ELSE 0 END) as passing, + SUM(CASE WHEN in_progress = 1 THEN 1 ELSE 0 END) as in_progress + FROM features + """) + row = cursor.fetchone() + total = row[0] or 0 + passing = row[1] or 0 + in_progress = row[2] or 0 + except sqlite3.OperationalError: + # Fallback for databases without in_progress column + cursor.execute(""" + SELECT + COUNT(*) as total, + SUM(CASE WHEN passes = 1 THEN 1 ELSE 0 END) as passing + FROM features + """) + row = cursor.fetchone() + total = row[0] or 0 + passing = row[1] or 0 + in_progress = 0 + return passing, in_progress, total except Exception as e: print(f"[Database error in count_passing_tests: {e}]") return 0, 0, 0 @@ -109,6 +129,8 @@ def get_all_passing_features(project_dir: Path) -> list[dict]: """ Get all passing features for webhook notifications. + Uses connection with proper timeout settings for parallel mode safety. + Args: project_dir: Directory containing the project @@ -120,17 +142,16 @@ def get_all_passing_features(project_dir: Path) -> list[dict]: return [] try: - conn = sqlite3.connect(db_file) - cursor = conn.cursor() - cursor.execute( - "SELECT id, category, name FROM features WHERE passes = 1 ORDER BY priority ASC" - ) - features = [ - {"id": row[0], "category": row[1], "name": row[2]} - for row in cursor.fetchall() - ] - conn.close() - return features + with closing(_get_connection(db_file)) as conn: + cursor = conn.cursor() + cursor.execute( + "SELECT id, category, name FROM features WHERE passes = 1 ORDER BY priority ASC" + ) + features = [ + {"id": row[0], "category": row[1], "name": row[2]} + for row in cursor.fetchall() + ] + return features except Exception: return [] @@ -233,21 +254,20 @@ def clear_stuck_features(project_dir: Path) -> int: return 0 try: - conn = sqlite3.connect(db_file) - cursor = conn.cursor() + with closing(_get_connection(db_file)) as conn: + cursor = conn.cursor() - # Count how many will be cleared - cursor.execute("SELECT COUNT(*) FROM features WHERE in_progress = 1") - count = cursor.fetchone()[0] + # Count how many will be cleared + cursor.execute("SELECT COUNT(*) FROM features WHERE in_progress = 1") + count = cursor.fetchone()[0] - if count > 0: - # Clear all in_progress flags - cursor.execute("UPDATE features SET in_progress = 0 WHERE in_progress = 1") - conn.commit() - print(f"[Auto-recovery] Cleared {count} stuck feature(s) from previous session") + if count > 0: + # Clear all in_progress flags + cursor.execute("UPDATE features SET in_progress = 0 WHERE in_progress = 1") + conn.commit() + print(f"[Auto-recovery] Cleared {count} stuck feature(s) from previous session") - conn.close() - return count + return count except sqlite3.OperationalError: # Table doesn't exist or doesn't have in_progress column return 0 diff --git a/quality_gates.py b/quality_gates.py index 927fc716..6f03e853 100644 --- a/quality_gates.py +++ b/quality_gates.py @@ -230,23 +230,39 @@ def run_type_check(project_dir: Path) -> QualityCheckResult: } -def run_custom_script(project_dir: Path, script_path: str | None = None) -> QualityCheckResult | None: +def run_custom_script( + project_dir: Path, + script_path: str | None = None, + explicit_config: bool = False, +) -> QualityCheckResult | None: """ Run a custom quality check script. Args: project_dir: Path to the project directory script_path: Path to the script (relative to project), defaults to .autocoder/quality-checks.sh + explicit_config: If True, user explicitly configured this script, so missing = error Returns: - QualityCheckResult, or None if script doesn't exist + QualityCheckResult, or None if default script doesn't exist """ + user_configured = script_path is not None or explicit_config + if script_path is None: script_path = ".autocoder/quality-checks.sh" script_full_path = project_dir / script_path if not script_full_path.exists(): + if user_configured: + # User explicitly configured a script that doesn't exist - return error + return { + "name": "custom_script", + "passed": False, + "output": f"Configured script not found: {script_path}", + "duration_ms": 0, + } + # Default script doesn't exist - that's OK, skip silently return None # Make sure it's executable @@ -309,7 +325,11 @@ def verify_quality( all_passed = False if run_custom: - custom_result = run_custom_script(project_dir, custom_script_path) + custom_result = run_custom_script( + project_dir, + custom_script_path, + explicit_config=custom_script_path is not None, + ) if custom_result is not None: checks["custom_script"] = custom_result if not custom_result["passed"]: From f7d9d15b0a872ed872a5861719f60bba416e01f6 Mon Sep 17 00:00:00 2001 From: cabana8471 Date: Tue, 27 Jan 2026 07:25:05 +0100 Subject: [PATCH 03/11] fix: correct import order per ruff (uppercase before lowercase) --- mcp_server/feature_mcp.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mcp_server/feature_mcp.py b/mcp_server/feature_mcp.py index fba4c4ba..88be6da6 100755 --- a/mcp_server/feature_mcp.py +++ b/mcp_server/feature_mcp.py @@ -41,7 +41,7 @@ # Add parent directory to path so we can import from api module sys.path.insert(0, str(Path(__file__).parent.parent)) -from api.database import atomic_transaction, create_database, Feature +from api.database import Feature, atomic_transaction, create_database from api.dependency_resolver import ( MAX_DEPENDENCIES_PER_FEATURE, compute_scheduling_scores, From 99f0ce37f9ee0f87fd37721729d116faba67943c Mon Sep 17 00:00:00 2001 From: cabana8471 Date: Tue, 27 Jan 2026 21:29:57 +0100 Subject: [PATCH 04/11] fix: use npx --no-install to prevent auto-download of tsc When tsc is not locally installed, npx without --no-install may prompt or auto-download the package. Use --no-install to fail fast instead, which is more predictable for quality gate checks. Addresses CodeRabbit review feedback. Co-Authored-By: Claude Opus 4.5 --- quality_gates.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/quality_gates.py b/quality_gates.py index 6f03e853..8d5433a0 100644 --- a/quality_gates.py +++ b/quality_gates.py @@ -139,7 +139,9 @@ def _detect_type_checker(project_dir: Path) -> tuple[str, list[str]] | None: if (project_dir / "node_modules/.bin/tsc").exists(): return ("tsc", ["node_modules/.bin/tsc", "--noEmit"]) if shutil.which("npx"): - return ("tsc", ["npx", "tsc", "--noEmit"]) + # Use --no-install to fail fast if tsc is not locally installed + # rather than prompting/auto-downloading + return ("tsc", ["npx", "--no-install", "tsc", "--noEmit"]) # Python (mypy) if (project_dir / "pyproject.toml").exists() or (project_dir / "setup.py").exists(): From 5c06163afe6536a288359f7c762cf59711011941 Mon Sep 17 00:00:00 2001 From: cabana8471 Date: Wed, 28 Jan 2026 06:36:50 +0100 Subject: [PATCH 05/11] fix: rename verify_quality parameters to avoid shadowing function names Parameters run_lint, run_type_check, run_custom shadowed the function names run_lint_check, run_type_check, run_custom_script, causing "'bool' object is not callable" errors at runtime. Renamed to do_lint, do_type_check, do_custom. Co-Authored-By: Claude Opus 4.5 --- mcp_server/feature_mcp.py | 12 ++++++------ quality_gates.py | 18 +++++++++--------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/mcp_server/feature_mcp.py b/mcp_server/feature_mcp.py index 88be6da6..0a3f0e74 100755 --- a/mcp_server/feature_mcp.py +++ b/mcp_server/feature_mcp.py @@ -255,9 +255,9 @@ def feature_verify_quality() -> str: checks_config = config.get("checks", {}) result = verify_quality( PROJECT_DIR, - run_lint=checks_config.get("lint", True), - run_type_check=checks_config.get("type_check", True), - run_custom=True, + do_lint=checks_config.get("lint", True), + do_type_check=checks_config.get("type_check", True), + do_custom=True, custom_script_path=checks_config.get("custom_script"), ) @@ -294,9 +294,9 @@ def feature_mark_passing( checks_config = config.get("checks", {}) quality_result = verify_quality( PROJECT_DIR, - run_lint=checks_config.get("lint", True), - run_type_check=checks_config.get("type_check", True), - run_custom=True, + do_lint=checks_config.get("lint", True), + do_type_check=checks_config.get("type_check", True), + do_custom=True, custom_script_path=checks_config.get("custom_script"), ) diff --git a/quality_gates.py b/quality_gates.py index 8d5433a0..29f041ea 100644 --- a/quality_gates.py +++ b/quality_gates.py @@ -293,9 +293,9 @@ def run_custom_script( def verify_quality( project_dir: Path, - run_lint: bool = True, - run_type_check: bool = True, - run_custom: bool = True, + do_lint: bool = True, + do_type_check: bool = True, + do_custom: bool = True, custom_script_path: str | None = None, ) -> QualityGateResult: """ @@ -303,9 +303,9 @@ def verify_quality( Args: project_dir: Path to the project directory - run_lint: Whether to run lint check - run_type_check: Whether to run type check - run_custom: Whether to run custom script + do_lint: Whether to run lint check + do_type_check: Whether to run type check + do_custom: Whether to run custom script custom_script_path: Path to custom script (optional) Returns: @@ -314,19 +314,19 @@ def verify_quality( checks: dict[str, QualityCheckResult] = {} all_passed = True - if run_lint: + if do_lint: lint_result = run_lint_check(project_dir) checks["lint"] = lint_result if not lint_result["passed"]: all_passed = False - if run_type_check: + if do_type_check: type_result = run_type_check(project_dir) checks["type_check"] = type_result if not type_result["passed"]: all_passed = False - if run_custom: + if do_custom: custom_result = run_custom_script( project_dir, custom_script_path, From 9a0202b97fb7e60200efa762def3119bbb71aff4 Mon Sep 17 00:00:00 2001 From: cabana8471 Date: Wed, 28 Jan 2026 22:26:07 +0100 Subject: [PATCH 06/11] fix: sort imports alphabetically to fix ruff I001 Co-Authored-By: Claude Opus 4.5 --- mcp_server/feature_mcp.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mcp_server/feature_mcp.py b/mcp_server/feature_mcp.py index 0a3f0e74..9a0be400 100755 --- a/mcp_server/feature_mcp.py +++ b/mcp_server/feature_mcp.py @@ -41,7 +41,7 @@ # Add parent directory to path so we can import from api module sys.path.insert(0, str(Path(__file__).parent.parent)) -from api.database import Feature, atomic_transaction, create_database +from api.database import atomic_transaction, create_database, Feature from api.dependency_resolver import ( MAX_DEPENDENCIES_PER_FEATURE, compute_scheduling_scores, From b4f4aae113880a6e76bb029213ff694db39a1c28 Mon Sep 17 00:00:00 2001 From: cabana8471 Date: Thu, 29 Jan 2026 07:56:01 +0100 Subject: [PATCH 07/11] fix: organize imports in feature_mcp.py Co-Authored-By: Claude Opus 4.5 --- mcp_server/feature_mcp.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mcp_server/feature_mcp.py b/mcp_server/feature_mcp.py index 9a0be400..0a3f0e74 100755 --- a/mcp_server/feature_mcp.py +++ b/mcp_server/feature_mcp.py @@ -41,7 +41,7 @@ # Add parent directory to path so we can import from api module sys.path.insert(0, str(Path(__file__).parent.parent)) -from api.database import atomic_transaction, create_database, Feature +from api.database import Feature, atomic_transaction, create_database from api.dependency_resolver import ( MAX_DEPENDENCIES_PER_FEATURE, compute_scheduling_scores, From 374a56522ba6f0c8cac8c06ab642c42607e15cea Mon Sep 17 00:00:00 2001 From: cabana8471 Date: Fri, 30 Jan 2026 23:07:49 +0100 Subject: [PATCH 08/11] fix: add UTF-8 encoding for subprocess calls on Windows Add explicit encoding="utf-8" and errors="replace" to subprocess.run calls in quality_gates.py to fix Windows CP1252 encoding issues. Closes #138 Co-Authored-By: Claude Opus 4.5 --- quality_gates.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/quality_gates.py b/quality_gates.py index 29f041ea..05534226 100644 --- a/quality_gates.py +++ b/quality_gates.py @@ -57,6 +57,8 @@ def _run_command(cmd: list[str], cwd: Path, timeout: int = 60) -> tuple[int, str cwd=cwd, capture_output=True, text=True, + encoding="utf-8", # Fix Windows CP1252 encoding issue (#138) + errors="replace", timeout=timeout, ) duration_ms = int((time.time() - start) * 1000) From 54b677bb02420ce439be25a9f9e75cde30c7b722 Mon Sep 17 00:00:00 2001 From: cabana8471 Date: Fri, 30 Jan 2026 23:37:23 +0100 Subject: [PATCH 09/11] fix: add Windows-friendly detection for local tool binaries Use shutil.which() with custom path parameter to detect executables in node_modules/.bin and venv directories across platforms. - ESLint/Biome: detect .cmd files on Windows via shutil.which() - Python venv: check venv/Scripts on Windows, venv/bin on Unix - TypeScript: same pattern for tsc detection - mypy: same pattern for venv detection Addresses CodeRabbit review feedback on PR #110. Co-Authored-By: Claude Opus 4.5 --- quality_gates.py | 58 ++++++++++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/quality_gates.py b/quality_gates.py index 05534226..70990af4 100644 --- a/quality_gates.py +++ b/quality_gates.py @@ -13,6 +13,7 @@ """ import json +import os import shutil import subprocess from datetime import datetime @@ -80,13 +81,17 @@ def _detect_js_linter(project_dir: Path) -> tuple[str, list[str]] | None: Returns: (name, command) tuple, or None if no linter detected """ - # Check for ESLint - if (project_dir / "node_modules/.bin/eslint").exists(): - return ("eslint", ["node_modules/.bin/eslint", ".", "--max-warnings=0"]) - - # Check for Biome - if (project_dir / "node_modules/.bin/biome").exists(): - return ("biome", ["node_modules/.bin/biome", "lint", "."]) + # Check for ESLint in node_modules/.bin (handles .cmd on Windows) + bin_dir = project_dir / "node_modules" / ".bin" + if bin_dir.exists(): + eslint = shutil.which("eslint", path=str(bin_dir)) + if eslint: + return ("eslint", [eslint, ".", "--max-warnings=0"]) + + # Check for Biome + biome = shutil.which("biome", path=str(bin_dir)) + if biome: + return ("biome", [biome, "lint", "."]) # Check for package.json lint script package_json = project_dir / "package.json" @@ -109,22 +114,24 @@ def _detect_python_linter(project_dir: Path) -> tuple[str, list[str]] | None: Returns: (name, command) tuple, or None if no linter detected """ - # Check for ruff + # Check for ruff in PATH if shutil.which("ruff"): return ("ruff", ["ruff", "check", "."]) - # Check for flake8 + # Check for flake8 in PATH if shutil.which("flake8"): return ("flake8", ["flake8", "."]) - # Check in virtual environment - venv_ruff = project_dir / "venv/bin/ruff" - if venv_ruff.exists(): - return ("ruff", [str(venv_ruff), "check", "."]) + # Check in virtual environment (venv/Scripts on Windows, venv/bin on Unix) + venv_bin = project_dir / "venv" / ("Scripts" if os.name == "nt" else "bin") + if venv_bin.exists(): + venv_ruff = shutil.which("ruff", path=str(venv_bin)) + if venv_ruff: + return ("ruff", [venv_ruff, "check", "."]) - venv_flake8 = project_dir / "venv/bin/flake8" - if venv_flake8.exists(): - return ("flake8", [str(venv_flake8), "."]) + venv_flake8 = shutil.which("flake8", path=str(venv_bin)) + if venv_flake8: + return ("flake8", [venv_flake8, "."]) return None @@ -136,10 +143,13 @@ def _detect_type_checker(project_dir: Path) -> tuple[str, list[str]] | None: Returns: (name, command) tuple, or None if no type checker detected """ - # TypeScript + # TypeScript - check for tsc in node_modules/.bin (handles .cmd on Windows) if (project_dir / "tsconfig.json").exists(): - if (project_dir / "node_modules/.bin/tsc").exists(): - return ("tsc", ["node_modules/.bin/tsc", "--noEmit"]) + bin_dir = project_dir / "node_modules" / ".bin" + if bin_dir.exists(): + tsc = shutil.which("tsc", path=str(bin_dir)) + if tsc: + return ("tsc", [tsc, "--noEmit"]) if shutil.which("npx"): # Use --no-install to fail fast if tsc is not locally installed # rather than prompting/auto-downloading @@ -149,9 +159,13 @@ def _detect_type_checker(project_dir: Path) -> tuple[str, list[str]] | None: if (project_dir / "pyproject.toml").exists() or (project_dir / "setup.py").exists(): if shutil.which("mypy"): return ("mypy", ["mypy", "."]) - venv_mypy = project_dir / "venv/bin/mypy" - if venv_mypy.exists(): - return ("mypy", [str(venv_mypy), "."]) + + # Check in virtual environment (venv/Scripts on Windows, venv/bin on Unix) + venv_bin = project_dir / "venv" / ("Scripts" if os.name == "nt" else "bin") + if venv_bin.exists(): + venv_mypy = shutil.which("mypy", path=str(venv_bin)) + if venv_mypy: + return ("mypy", [venv_mypy, "."]) return None From 8f98b51bc02a9488800f37df2c6e45d0d7edb3db Mon Sep 17 00:00:00 2001 From: cabana8471 Date: Sat, 31 Jan 2026 02:49:14 +0100 Subject: [PATCH 10/11] fix: resolve mypy type error in quality_gates.py - Fix unpacked dict entry type mismatch at line 410 by adding explicit casts to dict[str, Any] for both unpacked dictionaries Co-Authored-By: Claude Opus 4.5 --- quality_gates.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/quality_gates.py b/quality_gates.py index 70990af4..735711b1 100644 --- a/quality_gates.py +++ b/quality_gates.py @@ -18,7 +18,7 @@ import subprocess from datetime import datetime from pathlib import Path -from typing import TypedDict +from typing import Any, TypedDict, cast class QualityCheckResult(TypedDict): @@ -407,7 +407,7 @@ def load_quality_config(project_dir: Path) -> dict: result[key] = quality_config[key] if "checks" in quality_config: - result["checks"] = {**defaults["checks"], **quality_config["checks"]} + result["checks"] = {**cast(dict[str, Any], defaults["checks"]), **cast(dict[str, Any], quality_config["checks"])} return result except (json.JSONDecodeError, OSError): From 2390b4d73958a396f11f9d762652d767f379afc8 Mon Sep 17 00:00:00 2001 From: cabana8471 Date: Sat, 31 Jan 2026 09:14:51 +0100 Subject: [PATCH 11/11] fix: resolve CodeRabbit issues in quality_gates.py - Replace deprecated datetime.utcnow() with datetime.now(timezone.utc) - Add cross-platform Windows support for custom scripts (.bat, .cmd, .ps1) - Move `import time` from function body to module level Co-Authored-By: Claude Opus 4.5 --- quality_gates.py | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/quality_gates.py b/quality_gates.py index 735711b1..a727fee7 100644 --- a/quality_gates.py +++ b/quality_gates.py @@ -16,7 +16,8 @@ import os import shutil import subprocess -from datetime import datetime +import time +from datetime import datetime, timezone from pathlib import Path from typing import Any, TypedDict, cast @@ -49,7 +50,6 @@ def _run_command(cmd: list[str], cwd: Path, timeout: int = 60) -> tuple[int, str Returns: (exit_code, combined_output, duration_ms) """ - import time start = time.time() try: @@ -289,8 +289,28 @@ def run_custom_script( except OSError: pass + # Determine shell command based on platform and script extension + if os.name == "nt": # Windows + ext = script_full_path.suffix.lower() + if ext in (".bat", ".cmd"): + shell_cmd = ["cmd.exe", "/c", str(script_full_path)] + elif ext == ".ps1": + shell_cmd = ["powershell.exe", "-ExecutionPolicy", "Bypass", "-File", str(script_full_path)] + elif shutil.which("bash"): + # Git Bash or WSL available + shell_cmd = ["bash", str(script_full_path)] + else: + return { + "name": "custom_script", + "passed": False, + "output": f"Cannot run {script_full_path.suffix} script on Windows without bash. Install Git Bash or use .bat/.ps1 script.", + "duration_ms": 0, + } + else: # POSIX (Linux, macOS) + shell_cmd = ["bash", str(script_full_path)] + exit_code, output, duration_ms = _run_command( - ["bash", str(script_full_path)], + shell_cmd, project_dir, timeout=300, # 5 minutes for custom scripts ) @@ -365,7 +385,7 @@ def verify_quality( return { "passed": all_passed, - "timestamp": datetime.utcnow().isoformat(), + "timestamp": datetime.now(timezone.utc).isoformat(), "checks": checks, "summary": summary, }