feat: add Quality Gates to enforce lint/type-check before marking features #110
+809
−228
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tures Implements proposal from issue leonvanzyl#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 leonvanzyl#96 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a quality-gates subsystem and integrates it into the MCP server: new Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant MCP as MCP Server\n(mcp_server/feature_mcp.py)
participant QG as Quality Gates\n(quality_gates.py)
participant DB as SQLite DB
Client->>MCP: feature_verify_quality() or feature_mark_passing()
MCP->>QG: load_quality_config(project_dir)
MCP->>QG: verify_quality(do_lint?, do_type?, do_custom?)
QG->>QG: detect tools & run checks (lint, type, custom)
QG-->>MCP: QualityGateResult
alt strict mode && failed checks
MCP-->>Client: error (blocking)
else checks passed or non-blocking
MCP->>DB: BEGIN IMMEDIATE/EXCLUSIVE
MCP->>DB: atomic UPDATE/INSERT (mark passing/flags/deps/etc.)
DB-->>MCP: commit
MCP-->>Client: success (optional quality_result)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@mcp_server/feature_mcp.py`:
- Around line 293-323: The code runs load_quality_config and verify_quality
while a DB session/transaction is open, which can hold locks; move the quality
checks (load_quality_config and verify_quality) to execute before opening or
while the session is closed (or explicitly close the current session before
calling verify_quality and reopen afterwards). Specifically, call
load_quality_config(PROJECT_DIR) and run verify_quality(...) before you
fetch/modify the Feature object and before using session, or if a session must
exist, session.close() before verify_quality and then create a new session to
reload the Feature, update feature.passes/feature.in_progress and call
session.commit(); ensure you preserve and propagate quality_result into the
final result.
In `@progress.py`:
- Around line 235-256: The try/except block that opens the SQLite connection
(conn = sqlite3.connect(db_file)) can leak the DB handle on errors; update the
logic to ensure conn is always closed and transactions rolled back by using a
context manager (with sqlite3.connect(db_file) as conn:) or adding a finally
that calls conn.rollback() if needed and conn.close() if conn is defined, and
move cursor and commit/close logic inside that guarded scope; reference the
variables conn, cursor and the existing exception handlers to locate and replace
the current try/except so all error paths close the connection.
In `@quality_gates.py`:
- Around line 233-273: If a user explicitly configured a custom script
(script_path parameter not None) the function run_custom_script should treat a
missing file as a failing QualityCheckResult instead of returning None; change
the missing-file branch to return a QualityCheckResult dict with "name":
"custom_script", "passed": False, "output": a clear message that the configured
script (include script_path) is missing, and "duration_ms": 0 when script_path
was provided, while keeping the current behavior of returning None only when
script_path was None (i.e., the default script was not present); keep the rest
of run_custom_script (chmod, _run_command, truncation) unchanged.
cabana8471-arch
added a commit
to cabana8471-arch/autocoder
that referenced
this pull request
Jan 27, 2026
…yl#110) Combined quality gates with atomic SQL updates for parallel safety. - Quality checks run before marking features as passing - Atomic UPDATE prevents race conditions in parallel mode Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
cabana8471-arch
added a commit
to cabana8471-arch/autocoder
that referenced
this pull request
Jan 27, 2026
…nzyl#100, leonvanzyl#108, leonvanzyl#109, leonvanzyl#110 PR leonvanzyl#110 (Quality Gates): - 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 PR leonvanzyl#109 (Rate Limit): - Extract rate limit logic to shared rate_limit_utils.py module - Remove duplicated code from agent.py and test_agent.py PR leonvanzyl#108 (SQLite Parallel): - Sort imports alphabetically in feature_mcp.py PR leonvanzyl#100 (Config Diagnostics): - Add logger.warning for pkill_processes validation failures PR leonvanzyl#95 (Infrastructure Mock): - Add language tags to fenced code blocks in initializer template Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@mcp_server/feature_mcp.py`:
- Around line 30-51: The import block for the project-local module is not
alphabetically ordered causing a ruff I001; reorder the names imported from
api.database (atomic_transaction, create_database, Feature) into alphabetical
order (atomic_transaction, create_database, Feature) or simply run `ruff check
--fix mcp_server/feature_mcp.py` to auto-fix; ensure the reordered import
remains directly after the sys.path insertion and before other local imports so
import order is consistent with ruff expectations.
In `@quality_gates.py`:
- Around line 130-143: The npx invocation in _detect_type_checker should include
the --no-install flag to avoid implicit downloads in CI; update the branch that
returns ("tsc", ["npx", "tsc", "--noEmit"]) to instead return ("tsc", ["npx",
"--no-install", "tsc", "--noEmit"]) so npx fails if the binary isn't present
locally.
Comment on lines
30
to
51
| 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, | ||
| would_create_circular_dependency, | ||
| ) | ||
| from api.migration import migrate_json_to_sqlite | ||
| from quality_gates import load_quality_config, verify_quality |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's read the actual lines 30-51 from the file
head -n 51 mcp_server/feature_mcp.py | tail -n 22Repository: leonvanzyl/autocoder
Length of output: 743
🏁 Script executed:
#!/bin/bash
# Run ruff check on the file to see if there are import ordering issues
ruff check mcp_server/feature_mcp.py --select=IRepository: leonvanzyl/autocoder
Length of output: 1188
Fix import ordering to satisfy ruff (I001 error).
The import block at lines 44–51 is unsorted. Run ruff check --fix mcp_server/feature_mcp.py to automatically correct the import ordering, or manually sort the imports from api.database alphabetically.
🧰 Tools
🪛 GitHub Actions: CI
[error] 44-51: ruff check: Import block is unsorted or unformatted. Organize imports. 1 fixable with the --fix option. Command 'ruff check .' failed with exit code 1.
🤖 Prompt for AI Agents
In `@mcp_server/feature_mcp.py` around lines 30 - 51, The import block for the
project-local module is not alphabetically ordered causing a ruff I001; reorder
the names imported from api.database (atomic_transaction, create_database,
Feature) into alphabetical order (atomic_transaction, create_database, Feature)
or simply run `ruff check --fix mcp_server/feature_mcp.py` to auto-fix; ensure
the reordered import remains directly after the sys.path insertion and before
other local imports so import order is consistent with ruff expectations.
getworken
pushed a commit
to getworken/autocoder
that referenced
this pull request
Jan 27, 2026
getworken
pushed a commit
to getworken/autocoder
that referenced
this pull request
Jan 27, 2026
SUMMARY:
Fixed TypeScript build error in ProjectSetupRequired.tsx where startAgent
was being called with a boolean instead of an options object.
DETAILS:
- The startAgent API function signature was updated (in previous PR merges)
to accept an options object: { yoloMode?, parallelMode?, maxConcurrency?, testingAgentRatio? }
- ProjectSetupRequired.tsx was still calling it with the old signature:
startAgent(projectName, yoloMode) - passing boolean directly
- Changed to: startAgent(projectName, { yoloMode }) - wrapping in options object
This was the only remaining build error after merging 13+ PRs from upstream:
- PR leonvanzyl#112: Security vulnerabilities and race conditions
- PR leonvanzyl#89: Windows subprocess blocking fix
- PR leonvanzyl#109: Rate limit handling with exponential backoff
- PR leonvanzyl#88: MCP server config for ExpandChatSession
- PR leonvanzyl#100: Diagnostic warnings for config loading
- PR leonvanzyl#110: Quality gates (quality_gates.py)
- PR leonvanzyl#113: Structured logging (structured_logging.py)
- PR leonvanzyl#48: Knowledge files support (API, schemas, prompts)
- PR leonvanzyl#29: Feature editing/deletion (MCP tools)
- PR leonvanzyl#45: Chat persistence
- PR leonvanzyl#52: Refactoring feature guidance
- PR leonvanzyl#4: Project reset functionality
- PR leonvanzyl#95: UI polish, health checks, cross-platform fixes
Build now passes: npm run build succeeds with all 2245 modules transformed.
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 <noreply@anthropic.com>
cabana8471-arch
added a commit
to cabana8471-arch/autocoder
that referenced
this pull request
Jan 27, 2026
Includes npx --no-install fix to prevent auto-download of tsc. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Implements the Quality Gates proposal from issue #96. This system enforces code quality checks before features can be marked as passing.
.autocoder/quality-checks.shfeature_mark_passingif quality checks fail (default: enabled)New MCP Tools
feature_verify_quality- Run quality checks on demand and see resultsModified Behavior
feature_mark_passingnow runs quality checks before markingConfiguration
Create
.autocoder/config.jsonin the project directory:{ "quality_gates": { "enabled": true, "strict_mode": true, "checks": { "lint": true, "type_check": true, "custom_script": null } } }To disable (not recommended):
{"quality_gates": {"enabled": false}}Files Changed
quality_gates.pymcp_server/feature_mcp.pyfeature_verify_quality, modifiedfeature_mark_passingprogress.pyclear_stuck_features()for auto-recoveryTest plan
Addresses #96
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.