feat: add Quality Gates to enforce lint/type-check before marking features#110
feat: add Quality Gates to enforce lint/type-check before marking features#110cabana8471-arch wants to merge 11 commits intoAutoForgeAI:masterfrom
Conversation
…tures Implements proposal from issue AutoForgeAI#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 AutoForgeAI#96 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a quality-gates subsystem with a public Changes
Sequence Diagram(s)sequenceDiagram
autonumber
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() / 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
🧪 Generate unit tests (beta)
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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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.
…eAI#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>
…orgeAI#100, AutoForgeAI#108, AutoForgeAI#109, AutoForgeAI#110 PR AutoForgeAI#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 AutoForgeAI#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 AutoForgeAI#108 (SQLite Parallel): - Sort imports alphabetically in feature_mcp.py PR AutoForgeAI#100 (Config Diagnostics): - Add logger.warning for pkill_processes validation failures PR AutoForgeAI#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.
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 AutoForgeAI#112: Security vulnerabilities and race conditions
- PR AutoForgeAI#89: Windows subprocess blocking fix
- PR AutoForgeAI#109: Rate limit handling with exponential backoff
- PR AutoForgeAI#88: MCP server config for ExpandChatSession
- PR AutoForgeAI#100: Diagnostic warnings for config loading
- PR AutoForgeAI#110: Quality gates (quality_gates.py)
- PR AutoForgeAI#113: Structured logging (structured_logging.py)
- PR AutoForgeAI#48: Knowledge files support (API, schemas, prompts)
- PR AutoForgeAI#29: Feature editing/deletion (MCP tools)
- PR AutoForgeAI#45: Chat persistence
- PR AutoForgeAI#52: Refactoring feature guidance
- PR AutoForgeAI#4: Project reset functionality
- PR AutoForgeAI#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>
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>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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 AutoForgeAI#112: Security vulnerabilities and race conditions
- PR AutoForgeAI#89: Windows subprocess blocking fix
- PR AutoForgeAI#109: Rate limit handling with exponential backoff
- PR AutoForgeAI#88: MCP server config for ExpandChatSession
- PR AutoForgeAI#100: Diagnostic warnings for config loading
- PR AutoForgeAI#110: Quality gates (quality_gates.py)
- PR AutoForgeAI#113: Structured logging (structured_logging.py)
- PR AutoForgeAI#48: Knowledge files support (API, schemas, prompts)
- PR AutoForgeAI#29: Feature editing/deletion (MCP tools)
- PR AutoForgeAI#45: Chat persistence
- PR AutoForgeAI#52: Refactoring feature guidance
- PR AutoForgeAI#4: Project reset functionality
- PR AutoForgeAI#95: UI polish, health checks, cross-platform fixes
Build now passes: npm run build succeeds with all 2245 modules transformed.
…orgeAI#100, AutoForgeAI#108, AutoForgeAI#109, AutoForgeAI#110 PR AutoForgeAI#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 AutoForgeAI#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 AutoForgeAI#108 (SQLite Parallel): - Sort imports alphabetically in feature_mcp.py PR AutoForgeAI#100 (Config Diagnostics): - Add logger.warning for pkill_processes validation failures PR AutoForgeAI#95 (Infrastructure Mock): - Add language tags to fenced code blocks in initializer template Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add explicit encoding="utf-8" and errors="replace" to subprocess.run calls in quality_gates.py to fix Windows CP1252 encoding issues. Closes leonvanzyl#138 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@quality_gates.py`:
- Around line 83-154: The Windows compatibility issue: replace POSIX-only
existence checks for local binaries (e.g., "node_modules/.bin/eslint",
"node_modules/.bin/biome", "node_modules/.bin/tsc" and venv paths like
"venv/bin/ruff", "venv/bin/flake8", "venv/bin/mypy") with OS-aware lookups—use
shutil.which to resolve executables (so it finds .cmd/.exe on Windows) and
additionally check the virtualenv Scripts directory ("venv/Scripts/...") when
the global tool isn't found; update the detection logic in the JS
lint/type-check block (the ESLint/Biome and tsconfig/tsc checks),
_detect_python_linter, and _detect_type_checker to prefer shutil.which results
and fall back to project_dir/venv/Scripts/<tool> (and
project_dir/node_modules/.bin/<tool> as needed) so local Windows installs are
detected.
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 AutoForgeAI#110. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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 AutoForgeAI#110. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@quality_gates.py`:
- Line 368: Replace the deprecated datetime.utcnow() call with a UTC-aware
timestamp: change uses of datetime.utcnow().isoformat() to
datetime.now(datetime.timezone.utc).isoformat() (or
datetime.now(timezone.utc).isoformat()), and ensure the datetime.timezone (or
timezone) symbol is imported or referenced where the timestamp is constructed so
the value is timezone-aware and doesn't trigger DeprecationWarning.
- Around line 292-296: The call to _run_command currently hardcodes ["bash",
str(script_full_path)] which breaks on Windows; update the logic around the
invocation that prepares the command (referencing _run_command, script_full_path
and project_dir) to choose the shell based on platform and script type: on POSIX
keep bash/sh, on Windows detect if the script has a .ps1 extension and use
PowerShell (or use cmd.exe for .bat/.cmd), and fall back to locating "bash" with
shutil.which if Windows should support Git Bash/WSL; ensure the chosen shell and
arguments are constructed correctly before passing to _run_command and add a
small docstring or comment noting the platform behavior.
🧹 Nitpick comments (1)
quality_gates.py (1)
52-52: Consider movingimport timeto the top of the file.Placing imports inside functions is unconventional in Python. While it works, having all imports at the module level improves readability and makes dependencies immediately visible.
- 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 <noreply@anthropic.com>
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.