mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(gateway): enforce [task-id] prefix on every commit
ContentActions.commit was stripping user-supplied prefixes but never
re-adding the canonical one. Dev prompt promised auto-prefix; code
delivered nothing. Now every gateway commit lands with [task-id-short].
Format choice: simple [task_id[:8]] (8-char), matching the dev prompt
("Auto-prefixes [task-id]") and CLAUDE.md's documented commit format.
The legacy templates/git/commit.py uses the richer
[root_short:task_short] for the commit_for_task API path with full
traceability metadata; the gateway commit path is intentionally simpler
and stays aligned with the prompt-level promise.
This commit is contained in:
@@ -113,14 +113,18 @@ class ContentActions:
|
||||
remediate="call give_me_work() first",
|
||||
context_briefing={},
|
||||
)
|
||||
canonical_prefix = f"[{str(t.id)[:8]}]"
|
||||
final_message = f"{canonical_prefix} {subject}"
|
||||
commit_result = await self.git.commit(
|
||||
branch_name=t.branch_name,
|
||||
message=subject,
|
||||
message=final_message,
|
||||
task_id=t.id,
|
||||
files=files,
|
||||
)
|
||||
sha = commit_result.get("sha", "")
|
||||
await self.task.add_progress(t.id, agent_id, f"committed {sha[:8]}: {subject}")
|
||||
await self.task.add_progress(
|
||||
t.id, agent_id, f"committed {sha[:8]}: {final_message}"
|
||||
)
|
||||
return Envelope.ok(
|
||||
status=str(t.status),
|
||||
task_id=str(t.id),
|
||||
|
||||
@@ -0,0 +1,145 @@
|
||||
"""Every gateway commit message gets [task-id-short] prefix.
|
||||
|
||||
ContentActions.commit promises in the dev prompt that `[task-id]` is
|
||||
auto-prefixed onto every commit. The gateway strips any user-supplied
|
||||
prefix via `_TASK_ID_PREFIX_RE` but, before this guard, never re-added
|
||||
the canonical one. These tests pin the contract.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from roboco.services.gateway.content_actions import ContentActions, ContentActionsDeps
|
||||
|
||||
|
||||
def _make_deps(**overrides: AsyncMock) -> ContentActionsDeps:
|
||||
"""Build a ContentActionsDeps for tests; mirrors test_content_actions."""
|
||||
if "task" in overrides:
|
||||
task = overrides["task"]
|
||||
else:
|
||||
task = AsyncMock()
|
||||
task.get_active_task_for_agent.return_value = None
|
||||
|
||||
if "git" in overrides:
|
||||
git = overrides["git"]
|
||||
else:
|
||||
git = AsyncMock()
|
||||
git.commit.return_value = {"sha": "abc12345"}
|
||||
git.diff.return_value = ""
|
||||
|
||||
messaging = overrides.get("messaging", AsyncMock())
|
||||
a2a = overrides.get("a2a", AsyncMock())
|
||||
journal = overrides.get("journal", AsyncMock())
|
||||
workspace = overrides.get("workspace", AsyncMock())
|
||||
return ContentActionsDeps(
|
||||
task=task,
|
||||
git=git,
|
||||
messaging=messaging,
|
||||
a2a=a2a,
|
||||
journal=journal,
|
||||
workspace=workspace,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_commit_prefixes_with_task_id_short() -> None:
|
||||
"""Plain message gets `[task-id-short] ` prepended."""
|
||||
aid = uuid4()
|
||||
tid = uuid4()
|
||||
expected_prefix = f"[{str(tid)[:8]}]"
|
||||
|
||||
t = MagicMock(
|
||||
id=tid,
|
||||
assigned_to=aid,
|
||||
plan="x",
|
||||
status="in_progress",
|
||||
branch_name="feature/backend/abcd1234",
|
||||
)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get_active_task_for_agent.return_value = t
|
||||
git_svc = AsyncMock()
|
||||
git_svc.commit.return_value = {"sha": "deadbeef"}
|
||||
|
||||
deps = _make_deps(task=task_svc, git=git_svc)
|
||||
actions = ContentActions(deps)
|
||||
|
||||
await actions.commit(
|
||||
agent_id=aid,
|
||||
message="feat(api): add login endpoint for session bootstrap",
|
||||
)
|
||||
|
||||
git_svc.commit.assert_awaited()
|
||||
# git.commit takes keyword-only args (branch_name, message, task_id, files)
|
||||
msg = git_svc.commit.await_args.kwargs["message"]
|
||||
assert msg.startswith(expected_prefix), (
|
||||
f"expected {expected_prefix} prefix; got {msg!r}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_commit_strips_then_re_adds_prefix() -> None:
|
||||
"""If the agent supplied a wrong prefix, strip it and add the canonical one."""
|
||||
aid = uuid4()
|
||||
tid = uuid4()
|
||||
expected_prefix = f"[{str(tid)[:8]}]"
|
||||
|
||||
t = MagicMock(
|
||||
id=tid,
|
||||
assigned_to=aid,
|
||||
plan="x",
|
||||
status="in_progress",
|
||||
branch_name="feature/backend/abcd1234",
|
||||
)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get_active_task_for_agent.return_value = t
|
||||
git_svc = AsyncMock()
|
||||
git_svc.commit.return_value = {"sha": "deadbeef"}
|
||||
|
||||
deps = _make_deps(task=task_svc, git=git_svc)
|
||||
actions = ContentActions(deps)
|
||||
|
||||
await actions.commit(
|
||||
agent_id=aid,
|
||||
message="[wrong-id] feat(api): add login endpoint for session bootstrap",
|
||||
)
|
||||
|
||||
msg = git_svc.commit.await_args.kwargs["message"]
|
||||
assert msg.startswith(expected_prefix)
|
||||
assert "[wrong-id]" not in msg
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_commit_prefix_collapses_multiple_spaces() -> None:
|
||||
"""`[old] foo` should become `[new] foo`, not `[new] foo`."""
|
||||
aid = uuid4()
|
||||
tid = uuid4()
|
||||
expected_prefix = f"[{str(tid)[:8]}]"
|
||||
|
||||
t = MagicMock(
|
||||
id=tid,
|
||||
assigned_to=aid,
|
||||
plan="x",
|
||||
status="in_progress",
|
||||
branch_name="feature/backend/abcd1234",
|
||||
)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get_active_task_for_agent.return_value = t
|
||||
git_svc = AsyncMock()
|
||||
git_svc.commit.return_value = {"sha": "deadbeef"}
|
||||
|
||||
deps = _make_deps(task=task_svc, git=git_svc)
|
||||
actions = ContentActions(deps)
|
||||
|
||||
await actions.commit(
|
||||
agent_id=aid,
|
||||
message="[old] feat(api): add login endpoint for session bootstrap",
|
||||
)
|
||||
|
||||
msg = git_svc.commit.await_args.kwargs["message"]
|
||||
# Exactly one space after the prefix bracket
|
||||
assert msg.startswith(f"{expected_prefix} feat(api):"), (
|
||||
f"expected single-space prefix; got {msg!r}"
|
||||
)
|
||||
@@ -109,9 +109,11 @@ async def test_commit_no_active_task_returns_invalid_state() -> None:
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_commit_strips_existing_task_prefix() -> None:
|
||||
"""Agent-supplied [task-id] prefix is stripped before validation."""
|
||||
"""Agent-supplied [task-id] prefix is stripped before validation;
|
||||
the canonical [task-id-short] is re-applied before git.commit."""
|
||||
agent_id = uuid4()
|
||||
task_id = uuid4()
|
||||
expected_prefix = f"[{str(task_id)[:8]}]"
|
||||
task_obj = MagicMock(
|
||||
id=task_id, status="in_progress", branch_name="feature/backend/abc"
|
||||
)
|
||||
@@ -130,9 +132,11 @@ async def test_commit_strips_existing_task_prefix() -> None:
|
||||
body = env.as_dict()
|
||||
|
||||
assert body["error"] is None
|
||||
# The subject passed to git.commit should not include the prefix
|
||||
# The subject passed to git.commit gets the canonical prefix re-applied,
|
||||
# not the user-supplied [ABC12345].
|
||||
call_kwargs = git_svc.commit.call_args.kwargs
|
||||
assert not call_kwargs["message"].startswith("[")
|
||||
assert call_kwargs["message"].startswith(expected_prefix)
|
||||
assert "[ABC12345]" not in call_kwargs["message"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user