diff --git a/roboco/services/git.py b/roboco/services/git.py
index cc52efbf..543f563a 100644
--- a/roboco/services/git.py
+++ b/roboco/services/git.py
@@ -4364,24 +4364,56 @@ class GitService(BaseService):
body=body,
)
original = await self.get_current_branch(ws)
+ # A dirty tree is an agent's active workspace. Cutting the scaffold
+ # branch here would sweep their uncommitted work into the conventions
+ # commit — ``checkout `` no-ops or is refused, ``checkout -B
+ # `` carries the dirty change, and ``commit`` captures it,
+ # so the agent's in-progress edit rides a project-level PR they never
+ # intended and vanishes from their working tree. Refuse outright before
+ # any checkout touches the tree.
+ if not await self._working_tree_is_clean(ws):
+ return None
try:
- await self._commit_conventions_file(ws, base, spec)
+ if not await self._commit_conventions_file(ws, base, spec):
+ return None
return await self._push_and_open_conventions_pr(
project_slug, ws, base, spec
)
finally:
await self._run_git(ws, ["checkout", original], check=False)
+ async def _working_tree_is_clean(self, workspace: Path) -> bool:
+ """True iff ``git status --porcelain`` is empty (no staged/unstaged
+ changes, no untracked entries). Used to gate workspace-mutating
+ project-level ops (the conventions scaffold) away from an agent's
+ active dirty tree."""
+ result = await self._run_git(workspace, ["status", "--porcelain"])
+ return not result.stdout.strip()
+
async def _commit_conventions_file(
self, workspace: Path, base: str, spec: _ConventionsPr
- ) -> None:
+ ) -> bool:
+ """Commit the conventions file on the scaffold branch cut from ``base``.
+
+ Returns False when the scaffold cannot be safely cut from ``base`` (the
+ ``checkout `` with ``check=False`` failed — a missing base ref,
+ or a dirty tree that refused the switch). In that case ``checkout -B
+ `` would cut the scaffold from the *current* branch (the
+ agent's task branch), basing a project-level PR on the agent's work;
+ the caller refuses rather than commit on the wrong base.
+ """
await self._run_git(workspace, ["checkout", base], check=False)
+ # check=False swallows the failed checkout; verify we actually landed
+ # on base before cutting the scaffold from here.
+ if await self.get_current_branch(workspace) != base:
+ return False
await self._run_git(workspace, ["checkout", "-B", spec.branch])
target = workspace / ".roboco" / "conventions.yml"
target.parent.mkdir(parents=True, exist_ok=True)
target.write_text(spec.content)
await self._run_git(workspace, ["add", ".roboco/conventions.yml"])
await self._run_git(workspace, ["commit", "-m", spec.title])
+ return True
async def _push_and_open_conventions_pr(
self, project_slug: str, workspace: Path, base: str, spec: _ConventionsPr
diff --git a/tests/unit/services/test_git_conventions_pr_dirty.py b/tests/unit/services/test_git_conventions_pr_dirty.py
new file mode 100644
index 00000000..46be612f
--- /dev/null
+++ b/tests/unit/services/test_git_conventions_pr_dirty.py
@@ -0,0 +1,184 @@
+"""F051: open_conventions_pr must not operate on a dirty working tree.
+
+``open_conventions_pr`` cuts its scaffold branch in an agent's clone (or the
+project's shared ``workspace_path``). It does ``checkout `` with
+``check=False`` and then ``checkout -B ``. On a dirty tree the
+``checkout `` either no-ops (already on base) or is refused and silently
+swallowed; ``checkout -B `` then carries the agent's uncommitted
+work onto the scaffold branch, and the ``commit`` sweeps it into the
+project-level conventions commit — the agent's in-progress change is gone
+from their working tree and rides a PR they never intended. Refuse a dirty
+tree up front (return None, no checkout) so an active workspace is never
+touched.
+"""
+
+from __future__ import annotations
+
+import subprocess
+from typing import TYPE_CHECKING
+from unittest.mock import AsyncMock, MagicMock
+
+import pytest
+from roboco.services import git as git_module
+from roboco.services.git import GitService
+
+if TYPE_CHECKING:
+ from pathlib import Path
+
+_SCAFFOLD_BRANCH = "chore/roboco-conventions-scaffold"
+
+
+def _git(repo: Path, *args: str) -> subprocess.CompletedProcess[str]:
+ return subprocess.run(
+ ["git", *args], cwd=repo, check=True, capture_output=True, text=True
+ )
+
+
+def _init_repo(repo: Path) -> None:
+ repo.mkdir(parents=True, exist_ok=True)
+ _git(repo, "init", "-b", "master")
+ _git(repo, "config", "user.email", "t@example.com")
+ _git(repo, "config", "user.name", "T")
+ _git(repo, "config", "commit.gpgsign", "false")
+ (repo / "README.md").write_text("# r\n")
+ _git(repo, "add", "README.md")
+ _git(repo, "commit", "-m", "init")
+
+
+def _svc(monkeypatch: pytest.MonkeyPatch, repo: Path) -> GitService:
+ svc = GitService.__new__(GitService)
+ svc.session = AsyncMock()
+
+ project = MagicMock()
+ project.workspace_path = str(repo)
+ project.default_branch = "master"
+ project_service = MagicMock()
+ project_service.get_by_slug = AsyncMock(return_value=project)
+ monkeypatch.setattr(git_module, "get_project_service", lambda _s: project_service)
+ # No remote token → push/PR skipped; we only exercise the local commit path.
+ monkeypatch.setattr(svc, "_token_for_project", AsyncMock(return_value=None))
+ return svc
+
+
+def _branch_exists(repo: Path, branch: str) -> bool:
+ res = subprocess.run(
+ ["git", "rev-parse", "--verify", branch],
+ cwd=repo,
+ capture_output=True,
+ text=True,
+ check=False,
+ )
+ return res.returncode == 0
+
+
+@pytest.mark.asyncio
+async def test_dirty_tree_refused_before_any_checkout(
+ tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+ """A dirty working tree is the agent's active workspace — refuse, return
+ None, and leave the tree exactly as it was (no scaffold branch, dirty
+ change still uncommitted in the working tree)."""
+ repo = tmp_path / "repo"
+ _init_repo(repo)
+ # Dirty the tree: modify a tracked file (the agent's in-progress work).
+ (repo / "README.md").write_text("# dirty work in progress\n")
+
+ svc = _svc(monkeypatch, repo)
+
+ result = await svc.open_conventions_pr(
+ "g-proj", content="version: 1\n", title="scaffold", body="b"
+ )
+
+ # Refused.
+ assert result is None
+ # The scaffold branch was never created (no commit landed anywhere).
+ assert not _branch_exists(repo, _SCAFFOLD_BRANCH)
+ # The agent's dirty change is still in the working tree, uncommitted.
+ status = subprocess.run(
+ ["git", "status", "--porcelain"],
+ cwd=repo,
+ capture_output=True,
+ text=True,
+ check=True,
+ ).stdout
+ assert "README.md" in status
+ # master's history is untouched (still just the init commit).
+ log = subprocess.run(
+ ["git", "log", "--oneline", "master"],
+ cwd=repo,
+ capture_output=True,
+ text=True,
+ check=True,
+ ).stdout.strip()
+ assert log.count("\n") == 0 # exactly one commit on master
+
+
+@pytest.mark.asyncio
+async def test_clean_tree_proceeds_and_commits_on_scaffold(
+ tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+ """A clean tree proceeds: the conventions file is committed on the
+ scaffold branch cut from master, and master itself is untouched
+ (regression guard for the fix not over-rejecting the clean case)."""
+ repo = tmp_path / "repo"
+ _init_repo(repo)
+
+ svc = _svc(monkeypatch, repo)
+
+ result = await svc.open_conventions_pr(
+ "g-proj", content="version: 1\n", title="scaffold", body="b"
+ )
+
+ assert result is not None
+ assert result["branch"] == _SCAFFOLD_BRANCH
+ assert result["pr_number"] is None # no token → no remote PR
+ assert _branch_exists(repo, _SCAFFOLD_BRANCH)
+ show = subprocess.run(
+ ["git", "show", f"{_SCAFFOLD_BRANCH}:.roboco/conventions.yml"],
+ cwd=repo,
+ capture_output=True,
+ text=True,
+ check=False,
+ )
+ assert show.returncode == 0
+ assert show.stdout == "version: 1\n"
+ # The working tree ends back on master, clean.
+ assert (
+ subprocess.run(
+ ["git", "branch", "--show-current"],
+ cwd=repo,
+ capture_output=True,
+ text=True,
+ check=True,
+ ).stdout.strip()
+ == "master"
+ )
+
+
+@pytest.mark.asyncio
+async def test_missing_base_branch_refused(
+ tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+ """If ``checkout `` fails (base ref missing locally) the code must
+ not fall through to ``checkout -B `` from the current branch —
+ that would base the scaffold on the agent's task branch. Refuse when the
+ checkout doesn't actually land on base."""
+ repo = tmp_path / "repo"
+ _init_repo(repo)
+
+ svc = _svc(monkeypatch, repo)
+ # Override the project default to a branch that doesn't exist locally.
+ project = MagicMock()
+ project.workspace_path = str(repo)
+ project.default_branch = "nonexistent-base"
+ project_service = MagicMock()
+ project_service.get_by_slug = AsyncMock(return_value=project)
+ monkeypatch.setattr(git_module, "get_project_service", lambda _s: project_service)
+
+ result = await svc.open_conventions_pr(
+ "g-proj", content="version: 1\n", title="scaffold", body="b"
+ )
+
+ # Refused — no scaffold branch fabricated on top of master.
+ assert result is None
+ assert not _branch_exists(repo, _SCAFFOLD_BRANCH)