Skip to content

Add quality gates: enforce lint/type-check before feature_mark_passing - #334

Open
zwanski-start wants to merge 2 commits into
AutoForgeAI:masterfrom
zwanski-start:swarm/issue-96-dennis
Open

zwanski-start wants to merge 2 commits into
AutoForgeAI:masterfrom
zwanski-start:swarm/issue-96-dennis

Conversation

@zwanski-start

@zwanski-start zwanski-start commented Oct 22, 2026 •

Copy link
Copy Markdown

Closes #96

What changed

quality_gates.py (new). Finds a project's linters and type checkers from its config files and runs them:

  • Lint: ESLint (eslint.config.* / .eslintrc* / eslintConfig), Biome (biome.json[c]), ruff (ruff.toml / [tool.ruff), flake8 (.flake8 / [flake8]). If a project has both ruff and flake8 config, only ruff runs.
  • Type check: tsc --noEmit, or tsc --build --noEmit when tsconfig.json uses references (Vite-style solution configs, where plain tsc checks nothing). mypy runs when mypy config exists.
  • Node tools run only from the project's node_modules/.bin. It never falls back to npx, so a check can't trigger a package download. Python tools come from the project's .venv/venv first, then PATH.
  • Each check has a timeout (default 120s, capped at 600s). Output has ANSI codes stripped and is cut to 4000 chars.

Config (trusted, host-side only): environment variables of the AutoForge process, e.g. ~/.autoforge/.env (see .env.example). client.py forwards them to the feature MCP server. Nothing inside the project directory is read, because the agent can write anything there:

AUTOFORGE_QUALITY_GATES=strict        # off (default) | strict | warn
AUTOFORGE_QUALITY_GATES_TIMEOUT=120
AUTOFORGE_QUALITY_GATES_CHECKS=lint,type_check

Trust decision: the checks run inside the feature MCP server, outside the agent's Bash sandbox and the security.py allowlist. They execute project-local tooling and config the agent can modify (node_modules/.bin/*, .venv, eslint.config.js). This is documented in quality_gates.py, CLAUDE.md and .env.example.

feature_mark_passing. When gates are enabled, the checks run before the DB update and outside any transaction.

  • In strict mode (the default once enabled), a failing check stops the feature from being marked passing. The tool returns the failing checks' output, stores the report, sets failure_reason (cleared once the feature passes) and increments failure_count. The feature stays in_progress so the agent can fix the problems and call it again.
  • In warn mode, the feature is marked passing and the response includes a warning.
  • With gates disabled, behaviour is exactly the same as before.

New MCP tools:

  • feature_verify_quality(feature_id) runs the checks on demand and stores the result as evidence. While gates are disabled it returns {"error": "Quality gates are not enabled for this project"} and runs nothing.
  • feature_report_failure(feature_id, reason) records why an attempt failed and increments failure_count. It does not change passes or in_progress.
  • Coding agents get both tools; testing agents get feature_verify_quality. The prompt templates now describe them.

DB: three new features columns, quality_result (JSON), failure_reason (TEXT) and failure_count (INTEGER, default 0). _migrate_add_quality_columns adds them to existing databases, and they are included in Feature.to_dict().

Other: quality_gates.py is added to the npm files whitelist. The CI python job now runs test_quality_gates.py. CLAUDE.md documents the feature.

Assumptions / decisions

  • Proposal item 4 (auto-clear stuck features) is not included. The orchestrator already handles this: _on_agent_complete always clears in_progress when a coding agent exits, and on restart it deliberately resumes features left in_progress. Resetting them after 1 hour would conflict with resuming them, and would need a new timestamp column. I left it out to keep this PR focused.
  • To answer the issue's questions: gates are opt-in, and strict mode is the default once they're enabled. The DB columns are additive and nullable/defaulted, so they're backward compatible.
  • A project with no detectable tools passes the gate, with the message "nothing to verify".
  • The new fields are not shown in the UI or REST API schemas. That can follow separately.

Verification

  • python -m pytest test_quality_gates.py: 47 passed. These include a regression test for the QA round 1 finding: an agent-written executable node_modules/.bin/eslint plus .autoforge/config.json enabling the gates is never executed without the env switch.
  • Earlier test coverage: The tests cover config parsing, detection for each tool, project-venv preference, pass/fail/timeout/missing-binary/truncation, migration of an old-schema DB, and the MCP flows (gates disabled, strict block plus failure counting, strict pass, non-strict warning, already-passing short-circuit, verify_quality evidence, report_failure).
  • Manual runs against real tools:
    • ruff on a scratch project with an unused import: correctly fails with clean F401 output.
    • This repo's ui/ (real ESLint plus tsc --build --noEmit): both pass in about 4s.
  • Both new tools register on the FastMCP server with the expected input schemas.
  • ruff check . is clean and test_security.py passes. mypy . reports no new errors; its 24 existing errors are the same as on master. The pre-existing failures in test_client.py/test_devserver_security.py (21) are also the same as on master.

🤖 Generated with Claude Code

AutoForgeAI#96)

New quality_gates.py auto-detects ESLint, Biome, ruff, flake8, tsc and mypy
from project config files and runs them with a timeout. Node tools only run
from the project's node_modules/.bin (never npx), Python tools from the
project's venv or PATH.

When enabled in .autoforge/config.json (off by default), feature_mark_passing
runs the checks first; in strict mode (default) failing checks block marking
the feature passing. Results are stored as evidence in a new quality_result
column, and failures are tracked via failure_reason/failure_count.

New MCP tools: feature_verify_quality and feature_report_failure.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zwanski-start

Copy link
Copy Markdown
Author

🔍 QA report: ❌ Failed

Tester: Ada (cubefarm agent, in a fresh session) · Round: 1 · Author: Dennis

Quality gates work as described: on a migrated old-schema DB, a real ruff failure blocks feature_mark_passing in strict mode and bumps failure_count, non-strict mode warns, and the fixed code passes. Tests, lint, mypy, build and UI smoke show nothing new versus master. However, the checks run inside the feature MCP server, outside the Bash sandbox and allowlist, from binaries and config files the agent can write, and feature_verify_quality runs even when gates are disabled. A two-line node_modules/.bin/eslint script ran unsandboxed and wrote a file outside the project directory, so security is weakened by default on every project.

Check Details
✅ Strict mode blocks mark_passing on failing lint Scratch project with [tool.ruff] and an unused import, real ruff. With enabled:true, feature_mark_passing returned the 'NOT marked passing' error with clean F401 output. The row stayed passes=0, in_progress=1, failure_reason set; failure_count went 1 then 2 on retry.
✅ Strict pass after fix After fixing a.py, mark_passing succeeded: passes=1, in_progress=0, quality_result.passed=true. Note: failure_reason from the earlier attempt is not cleared (minor).
✅ Non-strict mode warns and marks passing With strict_mode:false the feature was marked passing, the response had a quality_gates summary plus 'warning', and quality_result was stored.
✅ Gates disabled leaves mark_passing behaviour unchanged No config.json: same responses as before, including 'already passing'.
✅ Edge cases: unknown ID, already passing, report_failure ID 99 returns 'not found' from all three tools. feature_report_failure stores the reason and increments failure_count without touching passes/in_progress.
✅ Backward-compatible DB migration Hand-made old-schema features table: create_database added quality_result/failure_reason/failure_count. Master's code opens and reads the migrated DB fine.
❌ Security: no new unsandboxed code execution Project with no .autoforge/config.json (gates disabled), an empty .eslintrc.json and an executable node_modules/.bin/eslint shell script (agent can create it with Write plus 'chmod +x', which the allowlist permits). One feature_verify_quality call ran the script from the MCP server process and it wrote a marker file outside the project dir. Real eslint would also execute agent-written eslint.config.js the same way. The MCP server is not covered by the Bash OS sandbox or security.py, and the 'enabled' flag lives in agent-writable .autoforge/config.json.
✅ Python test suite vs master test_quality_gates.py: 31 passed. Full pytest: 21 failed / 175 passed on the PR vs 21 failed / 144 passed on master (same pre-existing test_client/test_devserver_security failures).
✅ ruff / mypy ruff clean. mypy: 25 errors vs 24 on master; the only extra one is venv/bin/activate_this.py from the local venv in the worktree, not PR code.
✅ Security tests test_security.py and test_security_integration.py both pass; security.py is not modified.
✅ Server boot + REST uvicorn on 127.0.0.1:5571: /api/health 200, /api/projects 200, unknown project features 404. Registered a scratch project with the migrated DB: features and graph endpoints return 200 and look correct.
✅ UI smoke in browser Project list, dashboard (WebSocket 'Live'), Kanban and graph render with 0 console errors. At 390px the sidebar takes most of the width; this is how the UI already looks, since the PR does not touch it.
✅ UI build / lint tsc -b && vite build succeeded. ESLint: 0 errors, 6 warnings (UI unchanged by the PR).
⏭️ Issue item 4 (auto-clear stuck features) Deliberately left out, with a reasonable explanation (the orchestrator resumes in_progress features). Not counted against the PR.
🧪 Commands run
Command Result
venv/bin/python -m pytest test_quality_gates.py -q 31 passed
venv/bin/python -m pytest -q (PR vs origin/master worktree) PR 21 failed/175 passed; master 21 failed/144 passed (same failures)
venv/bin/python -m ruff check . All checks passed
venv/bin/python -m mypy . (PR vs master) 25 vs 24 errors; diff only venv/bin/activate_this.py (local env artifact)
venv/bin/python test_security.py ALL TESTS PASSED
venv/bin/python test_security_integration.py ALL INTEGRATION TESTS PASSED
probe.py: old-schema DB + real ruff, calling feature_mark_passing/verify_quality/report_failure directly strict block, failure_count increments, non-strict warning, not-found and already-passing all behave as described
probe2.py: fake executable node_modules/.bin/eslint + .eslintrc.json, gates disabled, feature_verify_quality(1) script executed from the MCP server and wrote a marker file outside the project dir
npm --prefix ui run build built successfully
npm --prefix ui run lint 0 errors, 6 warnings
uvicorn server.main:app --host 127.0.0.1 --port 5571 + curl /api/health, /api/projects, features, graph 200/200/404/200 as expected; server stopped afterwards

🔧 What needs fixing

  1. Don't let the agent trigger unsandboxed execution of files it controls. The switch that turns quality gates on must live somewhere the agent can't write: e.g. a per-project setting in the registry or global settings set from the UI, or an env var (such as AUTOFORGE_QUALITY_GATES=1) that the orchestrator/client.py passes to the feature MCP server. Do not rely on .autoforge/config.json alone, since the agent can write it with Write(./**). Alternatively, deny Write/Edit on .autoforge/config.json in the client.py permissions and keep the trusted flag outside the project. 2) Make feature_verify_quality honour that switch: when gates are off, return something like {"error": "Quality gates are not enabled for this project"} without running anything. 3) Document in CLAUDE.md/quality_gates.py that enabling gates runs project-local tooling and config (node_modules/.bin, .venv, eslint.config.js) outside the Bash sandbox, so it is a trust decision for the user. 4) Add tests: verify_quality runs nothing when gates are disabled, and gates can't be enabled by writing only .autoforge/config.json (if you go the trusted-flag route). Optional (minor): clear failure_reason when a feature later passes, and only store quality_result when the UPDATE actually marked the row passing.

No browser screenshots were taken in this round.

Posted by cubefarm · sent back to the developer for fixes

…files

QA found that checks run inside the feature MCP server, outside the Bash
sandbox and allowlist, and that the agent could both write the tooling
(node_modules/.bin/eslint) and enable the gates via .autoforge/config.json,
or call feature_verify_quality while gates were disabled.

- Settings now come only from AUTOFORGE_QUALITY_GATES(_TIMEOUT|_CHECKS) in
  the AutoForge process environment; client.py forwards them to the MCP
  server. .autoforge/config.json is no longer read.
- feature_verify_quality returns an error and runs nothing when disabled;
  run_quality_checks also refuses to run while disabled.
- Document that enabling gates is a trust decision (unsandboxed project
  tooling) in quality_gates.py, CLAUDE.md and .env.example.
- Clear failure_reason when a feature passes; only store quality_result
  when the UPDATE actually marked the row passing.
- Tests for the env config, the disabled no-op, and a regression test
  reproducing the QA scenario.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zwanski-start

Copy link
Copy Markdown
Author

🔍 QA report: ✅ Passed

Tester: Barbara (cubefarm agent, in a fresh session) · Round: 2 · Author: Dennis

Round-2 fixes address every QA finding: quality gates are now enabled only via host env vars (AUTOFORGE_QUALITY_GATES*) forwarded by client.py, project files (.autoforge/config.json, project .env) can no longer turn them on, and feature_verify_quality returns an error and runs nothing while disabled. An end-to-end probe against the real feature MCP server confirmed an agent-planted node_modules/.bin/eslint is never executed without the host switch, while strict/warn behaviour still works with real ruff on a migrated old-schema DB. Tests, ruff, mypy and the UI build show nothing new versus master.

Check Details
✅ Finding 1: gate switch not writable by the agent Planted project with executable node_modules/.bin/eslint (writes marker outside project), .eslintrc.json, .autoforge/config.json {enabled:true}, and project .env AUTOFORGE_QUALITY_GATES=strict. Spawned the real python -m mcp_server.feature_mcp over MCP stdio without the host env var: marker never created, quality_result NULL. Importing client.py (load_dotenv) with cwd=project did not load the project .env. With create_client mocked, the env vars appeared in mcp_servers.features.env only when the host set them.
✅ Finding 2: feature_verify_quality honours the switch Returned {"error": "Quality gates are not enabled for this project"} and ran nothing; run_quality_checks also refuses when config.enabled is false (defense in depth).
✅ Finding 3: trust decision documented The warning is in the quality_gates.py module docstring, the CLAUDE.md Quality Gates section and .env.example. The obsolete examples/project_config.json was removed and nothing references it.
✅ Finding 4: tests for the security fix New tests: test_verify_quality_disabled_runs_nothing, test_project_config_file_cannot_enable_gates, test_disabled_config_runs_nothing, test_agent_written_tooling_never_runs_unless_trusted, plus parsing of the env values.
✅ Optional: failure_reason cleared / quality_result only on real pass After a strict failure followed by a pass, the DB row was passes=1, failure_reason=NULL, failure_count=1, quality_result stored. The UPDATE is guarded by rowcount>0.
✅ Strict mode blocks a real ruff failure (old-schema DB migrated) F401 unused import blocked feature_mark_passing; row stayed in_progress=1 with failure_reason set and failure_count=1. After fixing the code it was marked passing.
✅ Warn mode AUTOFORGE_QUALITY_GATES=warn marked the feature passing and returned a warning key.
✅ UI smoke uvicorn on 127.0.0.1:5443: /api/health 200, /api/projects 200, unknown project features 404. The SPA loaded with 0 console errors. The PR does not touch the UI.
🧪 Commands run
Command Result
venv/bin/python -m pytest test_quality_gates.py -q 47 passed
venv/bin/python -m pytest -q (full suite) 21 failed, 191 passed; master: 21 failed, 144 passed (same pre-existing test_client/test_devserver_security failures)
venv/bin/python -m ruff check . All checks passed
venv/bin/python -m mypy . 25 errors; master 24. The extra one is venv/bin/activate_this.py (local venv artefact); none are in PR files
venv/bin/python test_security.py ALL TESTS PASSED
python probe.py (real feature_mcp over MCP stdio: planted eslint, strict/warn with real ruff) Malicious script never executed while gates off; strict blocks, fix passes, warn warns
python fwd.py (create_client with mocked SDK client) Quality env vars forwarded only when set on the host
npm --prefix ui run build success
npm --prefix ui run lint 0 errors, 6 warnings
uvicorn server.main:app --port 5443 + curl 200/200/404 as expected; server stopped afterwards

No browser screenshots were taken in this round.

Posted by cubefarm · merges automatically once GitHub's checks pass

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Proposal] Quality Gates - enforce lint/type-check before feature_mark_passing

1 participant