From e84a8432d912d15ba04b81e535c9be19ef29f05a Mon Sep 17 00:00:00 2001
From: Renn F <rennf93@users.noreply.github.com>
Date: Sun, 28 Jun 2026 10:39:24 +0200
Subject: [PATCH] [F012] release_executor: fail-closed on git add/commit before
 push

---
 roboco/services/release_executor.py           | 15 +++-
 ...est_release_executor_commit_fail_closed.py | 84 +++++++++++++++++++
 2 files changed, 97 insertions(+), 2 deletions(-)
 create mode 100644 tests/unit/services/test_release_executor_commit_fail_closed.py

diff --git a/roboco/services/release_executor.py b/roboco/services/release_executor.py
index fde310a4..bf1d589c 100644
--- a/roboco/services/release_executor.py
+++ b/roboco/services/release_executor.py
@@ -221,8 +221,19 @@ class _GitReleaseOps:
         return proc.returncode == 0
 
     async def commit_and_push(self, version: str) -> str:
-        await self._git("add", "-A")
-        await self._git("commit", "-S", "-m", f"chore(release): {version}")
+        add_rc, add_out = await self._git("add", "-A")
+        if add_rc != 0:
+            logger.error("release git add failed", error=add_out.strip()[:300])
+            raise RuntimeError(f"release git add failed: {add_out.strip()[:200]}")
+        commit_rc, commit_out = await self._git(
+            "commit", "-S", "-m", f"chore(release): {version}"
+        )
+        if commit_rc != 0:
+            # F012: a failed commit (gpgsign/pre-commit reject/no-op bump) must
+            # abort BEFORE rev-parse+push — otherwise the pre-bump base gets
+            # pushed and tagged as the new version.
+            logger.error("release commit failed", error=commit_out.strip()[:300])
+            raise RuntimeError(f"release commit failed: {commit_out.strip()[:200]}")
         _, out = await self._git("rev-parse", "HEAD")
         sha = out.strip()
         push_rc, push_out = await self._git(
diff --git a/tests/unit/services/test_release_executor_commit_fail_closed.py b/tests/unit/services/test_release_executor_commit_fail_closed.py
new file mode 100644
index 00000000..799bb837
--- /dev/null
+++ b/tests/unit/services/test_release_executor_commit_fail_closed.py
@@ -0,0 +1,84 @@
+"""F012 — ``_GitReleaseOps.commit_and_push`` must be fail-closed on commit.
+
+The release commit step discarded the ``git add`` / ``git commit`` return codes:
+on a failed commit (gpgsign unavailable, pre-commit hook rejection, nothing to
+commit after a no-op bump) the code still ran ``rev-parse HEAD`` + pushed the
+pre-bump base, so ``gh release create`` would tag the *old* tree as the new
+version. The fix checks both return codes and raises before any push.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+import pytest
+from roboco.services.release_executor import _GitReleaseOps, _ReleaseContext
+
+
+def _ctx() -> _ReleaseContext:
+    return _ReleaseContext(
+        slug="roboco",
+        default_branch="master",
+        root=Path("/tmp/roboco-release-f012"),
+        auth_url="https://x@github.com/o/roboco",
+        ci_workflow=None,
+    )
+
+
+class _FakeGitOps(_GitReleaseOps):
+    """Overrides ``_git`` with a scripted sequence of (rc, out) tuples."""
+
+    def __init__(self, ctx: _ReleaseContext, script: list[tuple[int, str]]) -> None:
+        # Bypass the real __init__ (no session needed) — we only exercise
+        # commit_and_push, which calls self._git.
+        self._slug = ctx.slug
+        self._default_branch = ctx.default_branch
+        self._root = ctx.root
+        self._auth_url = ctx.auth_url
+        self._ci_workflow = ctx.ci_workflow
+        self._script = list(script)
+        self.calls: list[tuple[str, ...]] = []
+
+    async def _git(self, *args: str) -> tuple[int, str]:  # type: ignore[override]
+        self.calls.append(args)
+        return self._script.pop(0)
+
+
+@pytest.mark.asyncio
+async def test_commit_failure_aborts_before_push() -> None:
+    """A failed ``git commit`` must raise — never push the pre-bump base."""
+    ops = _FakeGitOps(
+        _ctx(),
+        # add ok, commit FAILS (rc=1, e.g. gpgsign/pre-commit reject).
+        script=[(0, ""), (1, "error: gpg failed to sign the data")],
+    )
+    with pytest.raises(RuntimeError, match="commit"):
+        await ops.commit_and_push("0.13.0")
+    # rev-parse + push must never have run.
+    assert not any(c[:1] == ("rev-parse",) for c in ops.calls)
+    assert not any(c[:1] == ("push",) for c in ops.calls)
+
+
+@pytest.mark.asyncio
+async def test_add_failure_aborts_before_commit() -> None:
+    """A failed ``git add`` must raise before committing anything."""
+    ops = _FakeGitOps(_ctx(), script=[(1, "fatal: pathspec did not match")])
+    with pytest.raises(RuntimeError, match="add"):
+        await ops.commit_and_push("0.13.0")
+    assert not any(c[:1] == ("commit",) for c in ops.calls)
+    assert not any(c[:1] == ("push",) for c in ops.calls)
+
+
+@pytest.mark.asyncio
+async def test_green_commit_then_push_returns_sha() -> None:
+    """Happy path: add ok, commit ok, rev-parse sha, push ok → returns the sha."""
+    ops = _FakeGitOps(
+        _ctx(),
+        script=[(0, ""), (0, ""), (0, "deadbeef\n"), (0, "ok")],
+    )
+    sha = await ops.commit_and_push("0.13.0")
+    assert sha == "deadbeef"
+    assert ops.calls[0][:1] == ("add",)
+    assert ops.calls[1][:1] == ("commit",)
+    assert ops.calls[2][:1] == ("rev-parse",)
+    assert ops.calls[3][:1] == ("push",)