mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
feat(gateway): run a fast quality gate at i_am_done, before QA
The developer's i_am_done submit now runs the project's fast quality gate (lint + typecheck) in the developer's workspace and blocks the transition to awaiting_qa if it's red, returning the failing output as the remediate hint — so a red gate is caught at the dev's desk instead of in QA review or CI. The slow test suite intentionally stays on CI. The gate is fail-open on infrastructure errors (missing workspace/toolchain never blocks a submit) and a no-op for projects that configure no lint/typecheck commands. Developer prompt updated.
This commit is contained in:
Renn F
@@ -23,7 +23,7 @@ You write code; you do not coordinate. If you find yourself thinking "let me als
|
||||
| `commit(message)` | Makes the git commit, auto-prefixes `[task-id]`, records a progress entry. This is the ONLY way to commit — the gateway covers the actual git operation. | Task in `in_progress`; on your branch. |
|
||||
| `open_pr(task_id)` | Push your branch and open a PR. Run after your last commit, before `i_am_done`. `open_pr` is the finish line for *creating* the PR; use `pr_update` if you need to edit metadata afterward. | Task assigned to you; at least one commit; no PR yet. |
|
||||
| `pr_update(task_id, title?, body?, reviewers?)` | Update an existing PR's title, body, or reviewer list. Use after `open_pr` if you need to correct title/body or assign a reviewer. At least one field must be set. **Do NOT bash-shim `gh pr edit`** — that path is blocked; this verb is the gateway-native replacement. | Task has `pr_number`; you are the assignee (or your PM). |
|
||||
| `i_am_done(task_id, notes)` | Submit for QA. Auto-runs in_progress→verifying→awaiting_qa. Requires PR already open — run `open_pr` first. | At least one commit; PR open; progress entry; journal `reflect`; every acceptance criterion addressed. |
|
||||
| `i_am_done(task_id, notes)` | Submit for QA. Auto-runs in_progress→verifying→awaiting_qa. Requires PR already open — run `open_pr` first. Also runs your project's **fast quality gate (lint + typecheck) in your workspace and blocks the submit if it's red** — the failing output comes back in `remediate`; fix it, commit, and call again. | At least one commit; PR open; progress entry; journal `reflect`; every acceptance criterion addressed; lint + typecheck green. |
|
||||
| `i_am_blocked(task_id, reason, blocker_type?, what_needed?)` | Records the blocker, escalates to your PM, idles you. `blocker_type` ∈ `external` (waiting on a 3rd-party API/service), `internal` (a teammate or process), `question` (need clarification), `dependency` (waiting on another task). `what_needed` is a one-sentence concrete unblock request. Both fields are pre-gateway parity — PMs triage by class. | Task is yours and active. |
|
||||
| `unclaim(task_id)` | Release this claim back to pending. Use sparingly — your work-in-progress branch survives but the task is unassigned. | Task assigned to you and in claimed/in_progress. |
|
||||
| `resume(task_id)` | Resume a paused task. Transitions paused → in_progress. | Task assigned to you and in paused state. |
|
||||
@@ -101,7 +101,7 @@ The gateway enforces some of these; the rest are convention but failing one of t
|
||||
|
||||
1. ✅ At least one `commit()` on this branch (gateway-enforced).
|
||||
2. ✅ Every acceptance criterion is met by actual code or test, not just intention. Re-read them via `evidence(task_id)`.
|
||||
3. ✅ Tests/lint/typecheck pass locally — run them via `Bash`. If your project has `make quality` (or equivalent), run it; QA will run it too and fail you if it's red.
|
||||
3. ✅ Tests/lint/typecheck pass locally — run them via `Bash`. If your project has `make quality` (or equivalent), run it. **`i_am_done` runs the fast gate (lint + typecheck) in your workspace and rejects the submit if it's red** — so run it yourself first and submit green on the first try; QA and CI run the full gate (incl. tests) too.
|
||||
4. ✅ `git diff` (call `evidence(task_id)` to inspect) shows nothing stray — no `print()` debugging, no commented-out code, no unrelated edits.
|
||||
5. ✅ `note(scope='reflect', task_id=...)` walks through every criterion (gateway-enforced as `journal:reflect`).
|
||||
6. ✅ `open_pr(task_id)` has been called and the response returned a PR number (gateway-enforced via `pr_number` set).
|
||||
|
||||
@@ -1503,12 +1503,40 @@ class Choreographer:
|
||||
return await self._reject_i_am_done(ctx, rejection)
|
||||
if rejection := await self._ensure_branch_pushed(ctx):
|
||||
return await self._reject_i_am_done(ctx, rejection)
|
||||
if rejection := await self._check_quality_gate(ctx):
|
||||
return await self._reject_i_am_done(ctx, rejection)
|
||||
# Pre-gateway parity: persist per-criterion
|
||||
# status now that all gates have passed. The write runs AFTER the
|
||||
# verdict so it cannot change i_am_done's rejection behavior.
|
||||
await self._write_criteria_status(ctx.agent_id, ctx.task_id, ctx.task)
|
||||
return None
|
||||
|
||||
async def _check_quality_gate(self, ctx: _IAmDoneContext) -> Envelope | None:
|
||||
"""Run the project's fast quality gate (lint + typecheck) in the dev's
|
||||
workspace before the task reaches QA, so a red gate is caught at the
|
||||
dev's desk instead of in QA review or CI. The full test suite stays on
|
||||
CI. Fail-open: a gate-infrastructure error (missing workspace or
|
||||
toolchain) is logged and never blocks the submit; only an actual check
|
||||
failure blocks.
|
||||
"""
|
||||
try:
|
||||
result = await self.git.run_pre_submit_quality_gate(ctx.agent_id, ctx.task)
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"quality_gate_skipped", task_id=str(ctx.task_id), error=str(exc)
|
||||
)
|
||||
return None
|
||||
if result.passed:
|
||||
return None
|
||||
return Envelope.invalid_state(
|
||||
message=f"quality gate failed before QA — {result.summary}",
|
||||
remediate=(
|
||||
"Fix these in your workspace, commit, and call i_am_done again "
|
||||
"— QA reviews working code, not a red gate:\n\n" + result.output_excerpt
|
||||
),
|
||||
context_briefing=ctx.briefing,
|
||||
)
|
||||
|
||||
async def _ensure_branch_pushed(self, ctx: _IAmDoneContext) -> Envelope | None:
|
||||
"""Push the task branch to origin before it reaches awaiting_qa.
|
||||
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
"""Run a project's fast quality gate in a developer's workspace.
|
||||
|
||||
Invoked by the developer's ``i_am_done`` submit so a red gate is caught at the
|
||||
dev's desk instead of surfacing in QA review or CI. The gate runs the project's
|
||||
configured *non-mutating* fast checks (lint, typecheck); the slow test suite
|
||||
intentionally stays on CI. The gate is fail-open on infrastructure errors (a
|
||||
missing workspace or absent toolchain never blocks a submit) and fail-closed on
|
||||
an actual check failure.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
from dataclasses import dataclass
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from pathlib import Path
|
||||
|
||||
# A single check can be slow (mypy on a large tree); cap it generously but
|
||||
# never hang the submit verb forever.
|
||||
_GATE_TIMEOUT_SECONDS = 600
|
||||
# Cap the remediate excerpt so a huge lint dump doesn't bloat the envelope.
|
||||
_OUTPUT_EXCERPT_CHARS = 2000
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class GateResult:
|
||||
"""Outcome of a pre-submit quality gate run."""
|
||||
|
||||
passed: bool
|
||||
skipped: bool = False
|
||||
failures: tuple[str, ...] = ()
|
||||
output: str = ""
|
||||
|
||||
@property
|
||||
def summary(self) -> str:
|
||||
if self.skipped:
|
||||
return "no quality commands configured (skipped)"
|
||||
if self.passed:
|
||||
return "all checks passed"
|
||||
return f"failed: {', '.join(self.failures)}"
|
||||
|
||||
@property
|
||||
def output_excerpt(self) -> str:
|
||||
"""The tail of the combined output (where errors usually are)."""
|
||||
return self.output[-_OUTPUT_EXCERPT_CHARS:]
|
||||
|
||||
|
||||
async def run_quality_commands(
|
||||
workspace: Path, commands: list[tuple[str, str]]
|
||||
) -> GateResult:
|
||||
"""Run each ``(name, command)`` in ``workspace`` and aggregate the result.
|
||||
|
||||
Every command runs (we don't stop at the first failure) so the developer
|
||||
sees all gate failures in one shot. A non-zero exit is a failure; its
|
||||
combined stdout+stderr is captured for the remediate hint.
|
||||
"""
|
||||
if not commands:
|
||||
return GateResult(passed=True, skipped=True)
|
||||
failures: list[str] = []
|
||||
chunks: list[str] = []
|
||||
for name, command in commands:
|
||||
return_code, out = await _run_one(workspace, command)
|
||||
chunks.append(f"$ {command}\n{out.strip()}")
|
||||
if return_code != 0:
|
||||
failures.append(name)
|
||||
return GateResult(
|
||||
passed=not failures,
|
||||
failures=tuple(failures),
|
||||
output="\n\n".join(chunks),
|
||||
)
|
||||
|
||||
|
||||
async def _run_one(workspace: Path, command: str) -> tuple[int, str]:
|
||||
"""Run one operator-configured command string in the workspace shell."""
|
||||
proc = await asyncio.create_subprocess_shell(
|
||||
command,
|
||||
cwd=str(workspace),
|
||||
stdout=asyncio.subprocess.PIPE,
|
||||
stderr=asyncio.subprocess.STDOUT,
|
||||
)
|
||||
try:
|
||||
stdout, _ = await asyncio.wait_for(
|
||||
proc.communicate(), timeout=_GATE_TIMEOUT_SECONDS
|
||||
)
|
||||
except TimeoutError:
|
||||
proc.kill()
|
||||
return 124, f"command timed out after {_GATE_TIMEOUT_SECONDS}s"
|
||||
return proc.returncode or 0, stdout.decode("utf-8", errors="replace")
|
||||
@@ -48,6 +48,7 @@ from roboco.services.base import (
|
||||
UnauthorizedError,
|
||||
ValidationError,
|
||||
)
|
||||
from roboco.services.gateway.quality_gate import GateResult, run_quality_commands
|
||||
from roboco.services.project import get_project_service
|
||||
from roboco.services.task import TaskService, get_task_service
|
||||
from roboco.services.work_session import get_work_session_service
|
||||
@@ -1980,6 +1981,39 @@ class GitService(BaseService):
|
||||
return None
|
||||
return await project_service.get(project_ids[0])
|
||||
|
||||
@staticmethod
|
||||
def _fast_gate_commands(project: Any) -> list[tuple[str, str]]:
|
||||
"""The project's non-mutating fast-gate commands (lint + typecheck).
|
||||
|
||||
Format and the test suite are intentionally excluded: format mutates
|
||||
files, and the slow test run stays on CI.
|
||||
"""
|
||||
candidates = (
|
||||
("lint", getattr(project, "lint_command", None)),
|
||||
("typecheck", getattr(project, "typecheck_command", None)),
|
||||
)
|
||||
return [(name, cmd) for name, cmd in candidates if cmd]
|
||||
|
||||
async def run_pre_submit_quality_gate(
|
||||
self, actor_agent_id: UUID, task: Any
|
||||
) -> GateResult:
|
||||
"""Run the project's fast quality gate in the developer's workspace.
|
||||
|
||||
Resolves the task's project and the developer's workspace clone, then
|
||||
runs the configured lint + typecheck commands there. Returns a skipped
|
||||
pass when the project configures no fast-gate commands (so projects that
|
||||
opt out are never blocked). Raises only on workspace-resolution failure;
|
||||
the caller treats any such failure as fail-open.
|
||||
"""
|
||||
project = await self._project_for_task(task)
|
||||
if project is None:
|
||||
return GateResult(passed=True, skipped=True)
|
||||
commands = self._fast_gate_commands(project)
|
||||
if not commands:
|
||||
return GateResult(passed=True, skipped=True)
|
||||
workspace = await self.get_workspace(project.slug, actor_agent_id)
|
||||
return await run_quality_commands(workspace, commands)
|
||||
|
||||
async def _project_slug_for_branch(self, branch_name: str) -> str | None:
|
||||
"""Resolve project slug via the task that owns the branch."""
|
||||
task = await self._task_for_branch(branch_name)
|
||||
|
||||
@@ -0,0 +1,144 @@
|
||||
"""Pre-submit quality gate: lint + typecheck run in the dev's workspace on
|
||||
i_am_done, blocking a red submit before it reaches QA. Full tests stay on CI.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
|
||||
from roboco.services.gateway.choreographer._impl import _IAmDoneContext
|
||||
from roboco.services.gateway.quality_gate import GateResult, run_quality_commands
|
||||
from roboco.services.git import GitService
|
||||
|
||||
# --- the runner (real subprocess) -------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_commands_is_a_skipped_pass(tmp_path) -> None:
|
||||
result = await run_quality_commands(tmp_path, [])
|
||||
assert result.passed is True
|
||||
assert result.skipped is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_all_commands_pass(tmp_path) -> None:
|
||||
result = await run_quality_commands(
|
||||
tmp_path, [("lint", "echo lint-ok"), ("typecheck", "true")]
|
||||
)
|
||||
assert result.passed is True
|
||||
assert result.failures == ()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_failing_command_blocks_and_is_named(tmp_path) -> None:
|
||||
result = await run_quality_commands(
|
||||
tmp_path, [("lint", "echo problem-here; exit 1"), ("typecheck", "true")]
|
||||
)
|
||||
assert result.passed is False
|
||||
assert "lint" in result.failures
|
||||
assert "problem-here" in result.output
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_every_command_runs_even_after_a_failure(tmp_path) -> None:
|
||||
result = await run_quality_commands(
|
||||
tmp_path, [("lint", "echo AAA; exit 1"), ("typecheck", "echo BBB; exit 2")]
|
||||
)
|
||||
assert result.passed is False
|
||||
assert set(result.failures) == {"lint", "typecheck"}
|
||||
assert "AAA" in result.output
|
||||
assert "BBB" in result.output
|
||||
|
||||
|
||||
def test_gate_result_summary_and_excerpt() -> None:
|
||||
assert GateResult(passed=True).summary == "all checks passed"
|
||||
assert GateResult(passed=True, skipped=True).summary.startswith("no quality")
|
||||
failed = GateResult(passed=False, failures=("lint",), output="x" * 5000)
|
||||
assert "lint" in failed.summary
|
||||
# The excerpt is the truncated tail, shorter than the full output.
|
||||
assert 0 < len(failed.output_excerpt) < len(failed.output)
|
||||
|
||||
|
||||
# --- GitService command selection -------------------------------------------
|
||||
|
||||
|
||||
def test_fast_gate_commands_picks_lint_and_typecheck() -> None:
|
||||
project = SimpleNamespace(
|
||||
lint_command="uv run ruff check .",
|
||||
typecheck_command="uv run mypy roboco/",
|
||||
format_command="uv run ruff format .", # excluded (mutating)
|
||||
test_command="uv run pytest", # excluded (slow; CI only)
|
||||
)
|
||||
commands = GitService._fast_gate_commands(project)
|
||||
assert commands == [
|
||||
("lint", "uv run ruff check ."),
|
||||
("typecheck", "uv run mypy roboco/"),
|
||||
]
|
||||
|
||||
|
||||
def test_fast_gate_commands_empty_when_unconfigured() -> None:
|
||||
project = SimpleNamespace(lint_command=None, typecheck_command=None)
|
||||
assert GitService._fast_gate_commands(project) == []
|
||||
|
||||
|
||||
# --- choreographer glue -----------------------------------------------------
|
||||
|
||||
|
||||
def _choreo(mock_git: MagicMock) -> Choreographer:
|
||||
return Choreographer(
|
||||
ChoreographerDeps(
|
||||
task=MagicMock(),
|
||||
work_session=MagicMock(),
|
||||
git=mock_git,
|
||||
a2a=MagicMock(),
|
||||
journal=MagicMock(),
|
||||
audit=MagicMock(),
|
||||
evidence_repo=MagicMock(),
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
def _ctx() -> _IAmDoneContext:
|
||||
return _IAmDoneContext(
|
||||
agent_id=uuid4(),
|
||||
task_id=uuid4(),
|
||||
task=MagicMock(),
|
||||
role_str="developer",
|
||||
briefing={},
|
||||
notes="self-verification summary",
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_gate_pass_does_not_block() -> None:
|
||||
git = MagicMock()
|
||||
git.run_pre_submit_quality_gate = AsyncMock(return_value=GateResult(passed=True))
|
||||
assert await _choreo(git)._check_quality_gate(_ctx()) is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_gate_failure_blocks_with_output_in_remediate() -> None:
|
||||
git = MagicMock()
|
||||
git.run_pre_submit_quality_gate = AsyncMock(
|
||||
return_value=GateResult(
|
||||
passed=False, failures=("lint",), output="roboco/x.py:1 E501 line too long"
|
||||
)
|
||||
)
|
||||
env = await _choreo(git)._check_quality_gate(_ctx())
|
||||
assert env is not None
|
||||
assert env.error == "invalid_state"
|
||||
assert "E501" in (env.remediate or "")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_gate_is_fail_open_on_infrastructure_error() -> None:
|
||||
"""A missing workspace / toolchain must never block a submit."""
|
||||
git = MagicMock()
|
||||
git.run_pre_submit_quality_gate = AsyncMock(
|
||||
side_effect=RuntimeError("workspace not found")
|
||||
)
|
||||
assert await _choreo(git)._check_quality_gate(_ctx()) is None
|
||||
Reference in New Issue
Block a user