mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
feat(git): auto-regenerate + commit codegen drift before push (#632)
A project that checks in generated artifacts (RoboCo's lifecycle renders, verb tables) drifts whenever their source changes. The agent pre-submit gate (make gate) omits foundation-check, so drift is invisible at the desk and only fails on CI's drift gate — a failure with no link back to the task, which made one live task thrash 8 revision rounds. New per-project codegen_command (migration 078): run in the task's worktree right before push, and any drift committed into the same push, so CI never sees stale artifacts. Fail-open — a broken/timeout codegen command logs and lets the push proceed (CI's drift gate is the safety net); a null command (every project without checked-in codegen) is a pure no-op. Hooked at both push_branch (open_pr's first push, the PR head CI grades) and push_task_branch (later re-pushes). RoboCo sets codegen_command='make codegen' (a new Makefile target — the write counterpart to foundation-check's read) via the panel. Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
@@ -19,6 +19,7 @@ from roboco.config import settings
|
||||
from roboco.exceptions import GitCommandError, GitError, MergeConflictError
|
||||
from roboco.services.base import NotFoundError, UnauthorizedError, ValidationError
|
||||
from roboco.services.forge import RepoRef
|
||||
from roboco.services.gateway.quality_gate import GateResult
|
||||
from roboco.services.git import GitService
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@@ -163,6 +164,25 @@ async def test_push_task_branch_pushes_task_branch_by_name() -> None:
|
||||
assert_branch.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_push_task_branch_runs_codegen_before_push() -> None:
|
||||
"""push_task_branch regenerates + commits codegen drift before pushing."""
|
||||
task = MagicMock(branch_name="feature/backend/abc")
|
||||
project = MagicMock(slug="roboco")
|
||||
svc = _service()
|
||||
_bind(svc, "_assert_task_owned_with_branch", AsyncMock(return_value=task))
|
||||
_bind(svc, "_project_for_task", AsyncMock(return_value=project))
|
||||
_bind(svc, "get_workspace", AsyncMock(return_value=Path("/tmp/ws")))
|
||||
codegen_mock = AsyncMock()
|
||||
_bind(svc, "_run_codegen_and_commit", codegen_mock)
|
||||
push_mock = AsyncMock(return_value=("feature/backend/abc", _PUSHED_COMMIT_COUNT))
|
||||
_bind(svc, "push", push_mock)
|
||||
|
||||
await svc.push_task_branch(uuid4(), uuid4())
|
||||
|
||||
codegen_mock.assert_awaited_once_with("feature/backend/abc", Path("/tmp/ws"))
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_push_branch_pushes_named_branch_not_current_checkout() -> None:
|
||||
"""push_branch (open_pr's push side effect) pushes the NAMED branch.
|
||||
@@ -192,6 +212,23 @@ async def test_push_branch_pushes_named_branch_not_current_checkout() -> None:
|
||||
push_mock.assert_awaited_once_with(Path("/tmp/ws"), branch=branch_name)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_push_branch_runs_codegen_before_push() -> None:
|
||||
"""push_branch (open_pr's FIRST push) regenerates + commits codegen drift
|
||||
before pushing — the initial PR head must never carry stale artifacts."""
|
||||
branch_name = "feature/backend/abc"
|
||||
svc = _service()
|
||||
_bind(svc, "_workspace_for_branch", AsyncMock(return_value=Path("/tmp/ws")))
|
||||
codegen_mock = AsyncMock()
|
||||
_bind(svc, "_run_codegen_and_commit", codegen_mock)
|
||||
push_mock = AsyncMock(return_value=(branch_name, _PUSHED_COMMIT_COUNT))
|
||||
_bind(svc, "push", push_mock)
|
||||
|
||||
await svc.push_branch(branch_name)
|
||||
|
||||
codegen_mock.assert_awaited_once_with(branch_name, Path("/tmp/ws"))
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_push_targets_explicit_branch_not_current_checkout() -> None:
|
||||
"""push(branch=X) pushes X by ref even when the workspace is on Y."""
|
||||
@@ -367,6 +404,150 @@ async def test_push_task_branch_noop_for_project_less_task() -> None:
|
||||
push_mock.assert_not_awaited()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _run_codegen_and_commit: regenerate + commit codegen drift before push
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_codegen_command_for_returns_none_when_unset() -> None:
|
||||
project = MagicMock(codegen_command=None)
|
||||
assert GitService._codegen_command_for(project) is None
|
||||
|
||||
|
||||
def test_codegen_command_for_ignores_unspecced_mock_attribute() -> None:
|
||||
"""A bare MagicMock auto-vivifies codegen_command as a truthy MagicMock —
|
||||
the isinstance(str) guard must treat that the same as unset, or every
|
||||
loosely-specced GitService test would spuriously trip the codegen path."""
|
||||
assert GitService._codegen_command_for(MagicMock()) is None
|
||||
|
||||
|
||||
def test_codegen_command_for_returns_configured_command() -> None:
|
||||
project = MagicMock(codegen_command="make codegen")
|
||||
assert GitService._codegen_command_for(project) == "make codegen"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_codegen_and_commit_noop_when_command_unset() -> None:
|
||||
"""Null codegen_command (most projects) never runs a subprocess."""
|
||||
task = MagicMock(id=uuid4(), branch_name="feature/backend/abc")
|
||||
project = MagicMock(codegen_command=None)
|
||||
svc = _service()
|
||||
_bind(svc, "_task_for_branch", AsyncMock(return_value=task))
|
||||
_bind(svc, "_project_for_task", AsyncMock(return_value=project))
|
||||
run_mock = AsyncMock()
|
||||
with patch("roboco.services.git.run_quality_commands", run_mock):
|
||||
await svc._run_codegen_and_commit("feature/backend/abc", Path("/tmp/ws"))
|
||||
run_mock.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_codegen_and_commit_noop_when_no_task() -> None:
|
||||
svc = _service()
|
||||
_bind(svc, "_task_for_branch", AsyncMock(return_value=None))
|
||||
run_mock = AsyncMock()
|
||||
with patch("roboco.services.git.run_quality_commands", run_mock):
|
||||
await svc._run_codegen_and_commit("feature/backend/missing", Path("/tmp/ws"))
|
||||
run_mock.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_codegen_and_commit_commits_drift() -> None:
|
||||
"""Codegen runs clean but produces drift -> add -A + a task-prefixed commit."""
|
||||
task_id = uuid4()
|
||||
task = MagicMock(id=task_id, branch_name="feature/backend/abc")
|
||||
project = MagicMock(codegen_command="make codegen", slug="roboco")
|
||||
svc = _service()
|
||||
_bind(svc, "_task_for_branch", AsyncMock(return_value=task))
|
||||
_bind(svc, "_project_for_task", AsyncMock(return_value=project))
|
||||
_bind(svc, "_ensure_worktree_for_commit", AsyncMock())
|
||||
calls: list[list[str]] = []
|
||||
|
||||
async def _run_git(_ws: object, args: list[str], **_kw: object) -> MagicMock:
|
||||
calls.append(args)
|
||||
res = MagicMock()
|
||||
res.returncode = 0
|
||||
res.stdout = " M docs/rag/lifecycle/foo.md\n" if args[0] == "status" else ""
|
||||
return res
|
||||
|
||||
_bind(svc, "_run_git", AsyncMock(side_effect=_run_git))
|
||||
gate_result = GateResult(passed=True, output="ok")
|
||||
with patch(
|
||||
"roboco.services.git.run_quality_commands",
|
||||
AsyncMock(return_value=gate_result),
|
||||
):
|
||||
await svc._run_codegen_and_commit("feature/backend/abc", Path("/tmp/ws"))
|
||||
|
||||
assert ["add", "-A"] in calls
|
||||
commit_call = next(c for c in calls if c[0] == "commit")
|
||||
assert commit_call == [
|
||||
"commit",
|
||||
"-m",
|
||||
f"[{str(task_id)[:8]}] regenerate generated artifacts",
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_codegen_and_commit_noop_when_codegen_clean() -> None:
|
||||
"""Codegen runs clean with NO drift -> git status is clean, no commit."""
|
||||
task = MagicMock(id=uuid4(), branch_name="feature/backend/abc")
|
||||
project = MagicMock(codegen_command="make codegen", slug="roboco")
|
||||
svc = _service()
|
||||
_bind(svc, "_task_for_branch", AsyncMock(return_value=task))
|
||||
_bind(svc, "_project_for_task", AsyncMock(return_value=project))
|
||||
_bind(svc, "_ensure_worktree_for_commit", AsyncMock())
|
||||
calls: list[list[str]] = []
|
||||
|
||||
async def _run_git(_ws: object, args: list[str], **_kw: object) -> MagicMock:
|
||||
calls.append(args)
|
||||
res = MagicMock()
|
||||
res.returncode = 0
|
||||
res.stdout = ""
|
||||
return res
|
||||
|
||||
_bind(svc, "_run_git", AsyncMock(side_effect=_run_git))
|
||||
gate_result = GateResult(passed=True, output="ok")
|
||||
with patch(
|
||||
"roboco.services.git.run_quality_commands",
|
||||
AsyncMock(return_value=gate_result),
|
||||
):
|
||||
await svc._run_codegen_and_commit("feature/backend/abc", Path("/tmp/ws"))
|
||||
|
||||
assert not any(c[0] in ("add", "commit") for c in calls)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_codegen_and_commit_fail_open_on_command_failure() -> None:
|
||||
"""A non-zero codegen exit logs a warning and skips the commit (fail-open) —
|
||||
push must proceed rather than a broken codegen command blocking delivery."""
|
||||
task = MagicMock(id=uuid4(), branch_name="feature/backend/abc")
|
||||
project = MagicMock(codegen_command="make codegen", slug="roboco")
|
||||
svc = _service()
|
||||
_bind(svc, "_task_for_branch", AsyncMock(return_value=task))
|
||||
_bind(svc, "_project_for_task", AsyncMock(return_value=project))
|
||||
_bind(svc, "_ensure_worktree_for_commit", AsyncMock())
|
||||
run_git_mock = AsyncMock()
|
||||
_bind(svc, "_run_git", run_git_mock)
|
||||
gate_result = GateResult(passed=False, failures=("codegen",), output="boom")
|
||||
with patch(
|
||||
"roboco.services.git.run_quality_commands",
|
||||
AsyncMock(return_value=gate_result),
|
||||
):
|
||||
await svc._run_codegen_and_commit("feature/backend/abc", Path("/tmp/ws"))
|
||||
|
||||
# Never even checks git status — a failed codegen command skips straight
|
||||
# through without touching git.
|
||||
run_git_mock.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_codegen_and_commit_fail_open_on_exception() -> None:
|
||||
"""Any unexpected exception (worktree resolution, git failure, ...) must
|
||||
never raise out of this helper and break the caller's push."""
|
||||
svc = _service()
|
||||
_bind(svc, "_task_for_branch", AsyncMock(side_effect=RuntimeError("boom")))
|
||||
await svc._run_codegen_and_commit("feature/backend/abc", Path("/tmp/ws"))
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# diff: derives parent + invokes git diff
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user