diff --git a/backend/app/api/agent_forgejo.py b/backend/app/api/agent_forgejo.py index c54e101..495ac99 100644 --- a/backend/app/api/agent_forgejo.py +++ b/backend/app/api/agent_forgejo.py @@ -9,16 +9,24 @@ from fastapi import APIRouter, Depends, HTTPException, Query, status from sqlmodel import select, func from sqlalchemy import and_ -from app.api.deps import ActorContext, get_board_for_actor_read +from app.api.deps import get_board_for_actor_read from app.core.agent_auth import get_agent_auth_context from app.db.session import get_session +from app.models.boards import Board from app.models.board_repository_links import BoardRepositoryLink from app.models.forgejo_issues import ForgejoIssue from app.schemas.forgejo_issues import ForgejoIssueRead, ForgejoIssueListResponse, CloseIssueResponse +from app.services.activity_log import record_activity +from app.services.forgejo_issue_close import ( + CloseIssueAccessError, + CloseIssueNotFoundError, + CloseIssueRemoteError, + close_issue_by_id, +) +from app.services.openclaw.policies import OpenClawAuthorizationPolicy if TYPE_CHECKING: from sqlmodel.ext.asyncio.session import AsyncSession - from app.models.agents import Agent from app.core.agent_auth import AgentAuthContext @@ -248,10 +256,6 @@ async def read_board_issue( return ForgejoIssueRead.model_validate(issue) -from app.schemas.forgejo_issues import CloseIssueResponse -from app.services.forgejo_issue_close import close_issue_by_id, CloseIssueNotFoundError, CloseIssueAccessError, CloseIssueRemoteError - - @router.post( "/{board_id}/git/issues/{issue_id}/close", response_model=CloseIssueResponse, @@ -318,7 +322,7 @@ async def close_board_issue( board_id: UUID, issue_id: UUID, session: AsyncSession = SESSION_DEP, - board: ForgejoIssue = BOARD_READ_DEP, + board: Board = BOARD_READ_DEP, agent_ctx: AgentAuthContext = AGENT_CTX_DEP, ) -> CloseIssueResponse: """Close a Forgejo issue as a board-lead agent. @@ -326,18 +330,26 @@ async def close_board_issue( Only board lead agents can close issues. The issue must belong to a repository linked to the target board. """ - # Get the issue + links = ( + await session.exec( + select(BoardRepositoryLink).where( + BoardRepositoryLink.organization_id == board.organization_id, + BoardRepositoryLink.board_id == board_id, + ) + ) + ).all() + repository_ids = [link.repository_id for link in links] + if not repository_ids: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail="Issue not found or not linked to this board", + ) + statement = select(ForgejoIssue).where( and_( ForgejoIssue.id == issue_id, - ForgejoIssue.repository_id.in_( - [link.repository_id for link in await session.exec( - select(BoardRepositoryLink).where( - BoardRepositoryLink.board_id == board_id - ) - ).all()] - ), - ) + ForgejoIssue.repository_id.in_(repository_ids), + ), ) issue = (await session.exec(statement)).first() @@ -347,14 +359,13 @@ async def close_board_issue( detail="Issue not found or not linked to this board", ) - # Verify agent is board lead - from app.core.openclaw.policies import OpenClawAuthorizationPolicy + # Verify agent is board lead. OpenClawAuthorizationPolicy.require_board_lead_actor( actor_agent=agent_ctx.agent, detail="Only board leads can close issues", ) - # Close the issue using the service + # Close the issue using the service. try: result = await close_issue_by_id( session=session, @@ -368,6 +379,19 @@ async def close_board_issue( except CloseIssueRemoteError as e: raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail=str(e)) + repository_full_name = str(result.get("repository_full_name") or "unknown/unknown") + record_activity( + session, + event_type="forgejo.issue.closed", + message=( + "Forgejo issue closed by agent " + f"{agent_ctx.agent.id}: {repository_full_name}#{result['forgejo_issue_number']}" + ), + board_id=board_id, + agent_id=agent_ctx.agent.id, + ) + await session.commit() + return CloseIssueResponse( success=result["success"], issue_id=result["issue_id"], diff --git a/backend/app/api/forgejo_issues.py b/backend/app/api/forgejo_issues.py index 263071d..8986445 100644 --- a/backend/app/api/forgejo_issues.py +++ b/backend/app/api/forgejo_issues.py @@ -9,15 +9,16 @@ from fastapi import APIRouter, Depends, HTTPException, Query, status from sqlalchemy import cast, String from sqlmodel import select, func -from app.api.deps import get_board_for_user_write, require_org_member +from app.api.deps import require_org_member from app.core.auth import AuthContext, get_auth_context from app.db import crud from app.db.session import get_session from app.models.board_repository_links import BoardRepositoryLink from app.models.forgejo_issues import ForgejoIssue from app.schemas.forgejo_issues import ForgejoIssueListResponse, ForgejoIssueRead, CloseIssueResponse +from app.services.activity_log import record_activity from app.services.forgejo_issue_close import close_issue_by_id, CloseIssueNotFoundError, CloseIssueAccessError, CloseIssueRemoteError -from app.services.organizations import OrganizationContext +from app.services.organizations import OrganizationContext, list_accessible_board_ids if TYPE_CHECKING: from sqlmodel.ext.asyncio.session import AsyncSession @@ -235,28 +236,40 @@ async def close_issue( issue = await crud.get_by_id(session, ForgejoIssue, uuid) if issue is None: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Issue not found") + if issue.organization_id != ctx.organization.id: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Issue not found") + if auth.user is None: + raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED) - # Get the board linked to this issue's repository - link_statement = select(BoardRepositoryLink).where( - BoardRepositoryLink.repository_id == issue.repository_id, - ) - link = (await session.exec(link_statement)).first() - if link is None: + # Get boards linked to this issue's repository for this organization. + links = ( + await session.exec( + select(BoardRepositoryLink).where( + BoardRepositoryLink.organization_id == ctx.organization.id, + BoardRepositoryLink.repository_id == issue.repository_id, + ) + ) + ).all() + if not links: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail="Issue repository is not linked to any board", ) - # Verify the user has write access to the board - await get_board_for_user_write( - board_id=str(link.board_id), - session=session, - auth=auth, + allowed_board_ids = set( + await list_accessible_board_ids(session, member=ctx.member, write=True) ) - if auth.user is None: - raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED) + authorized_board_id = next( + (link.board_id for link in links if link.board_id in allowed_board_ids), + None, + ) + if authorized_board_id is None: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Board access denied", + ) - # Close the issue using the service + # Close the issue using the service. try: result = await close_issue_by_id( session=session, @@ -270,6 +283,18 @@ async def close_issue( except CloseIssueRemoteError as e: raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail=str(e)) + repository_full_name = str(result.get("repository_full_name") or "unknown/unknown") + record_activity( + session, + event_type="forgejo.issue.closed", + message=( + "Forgejo issue closed by user " + f"{auth.user.id}: {repository_full_name}#{result['forgejo_issue_number']}" + ), + board_id=authorized_board_id, + ) + await session.commit() + return CloseIssueResponse( success=result["success"], issue_id=result["issue_id"], diff --git a/backend/app/services/forgejo_issue_close.py b/backend/app/services/forgejo_issue_close.py index 07504b8..07c4043 100644 --- a/backend/app/services/forgejo_issue_close.py +++ b/backend/app/services/forgejo_issue_close.py @@ -78,7 +78,7 @@ async def close_cached_issue( # Close issue on Forgejo first try: async with get_forgejo_client(connection) as client: - result = await client.close_issue( + await client.close_issue( owner=repository.owner, repo=repository.repo, issue_number=issue.forgejo_issue_number, @@ -111,6 +111,8 @@ async def close_cached_issue( "success": True, "issue_id": str(issue.id), "forgejo_issue_number": issue.forgejo_issue_number, + "repository_id": str(repository.id), + "repository_full_name": f"{repository.owner}/{repository.repo}", "state": "closed", "forgejo_closed_at": issue.forgejo_closed_at.isoformat() if issue.forgejo_closed_at else None, "last_synced_at": issue.last_synced_at.isoformat(), diff --git a/backend/tests/test_forgejo_issue_close_api.py b/backend/tests/test_forgejo_issue_close_api.py new file mode 100644 index 0000000..0ad3d7b --- /dev/null +++ b/backend/tests/test_forgejo_issue_close_api.py @@ -0,0 +1,407 @@ +# ruff: noqa: INP001 +"""Integration tests for Forgejo issue close endpoints.""" + +from __future__ import annotations + +from datetime import datetime +from types import SimpleNamespace +from uuid import uuid4 + +import pytest +from fastapi import APIRouter, FastAPI +from httpx import ASGITransport, AsyncClient +from sqlalchemy.ext.asyncio import AsyncEngine, async_sessionmaker, create_async_engine +from sqlmodel import SQLModel, select +from sqlmodel.ext.asyncio.session import AsyncSession + +from app import models as _models +from app.api import deps as deps_api +from app.api.agent_forgejo import router as agent_forgejo_router +from app.api.forgejo_issues import router as forgejo_issues_router +from app.api.deps import require_org_member +from app.core.agent_auth import AgentAuthContext, get_agent_auth_context +from app.core.auth import AuthContext, get_auth_context +from app.db.session import get_session +from app.models.activity_events import ActivityEvent +from app.models.agents import Agent +from app.models.board_repository_links import BoardRepositoryLink +from app.models.boards import Board +from app.models.forgejo_connections import ForgejoConnection +from app.models.forgejo_issues import ForgejoIssue +from app.models.forgejo_repositories import ForgejoRepository +from app.models.gateways import Gateway +from app.models.organization_board_access import OrganizationBoardAccess +from app.models.organization_members import OrganizationMember +from app.models.organizations import Organization +from app.models.users import User +from app.services import forgejo_issue_close +from app.services.organizations import OrganizationContext + + +async def _make_engine() -> AsyncEngine: + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + async with engine.begin() as conn: + await conn.run_sync(SQLModel.metadata.create_all) + return engine + + +def _build_test_app( + session_maker: async_sessionmaker[AsyncSession], + *, + org_ctx: OrganizationContext | None = None, + auth_ctx: AuthContext | None = None, + agent_ctx: AgentAuthContext | None = None, + board: Board | None = None, +) -> FastAPI: + app = FastAPI() + api_v1 = APIRouter(prefix="/api/v1") + api_v1.include_router(forgejo_issues_router) + api_v1.include_router(agent_forgejo_router) + app.include_router(api_v1) + + async def _override_get_session() -> AsyncSession: + async with session_maker() as session: + yield session + + app.dependency_overrides[get_session] = _override_get_session + + if org_ctx is not None: + async def _override_require_org_member() -> OrganizationContext: + return org_ctx + + app.dependency_overrides[require_org_member] = _override_require_org_member + + if auth_ctx is not None: + async def _override_get_auth_context() -> AuthContext: + return auth_ctx + + app.dependency_overrides[get_auth_context] = _override_get_auth_context + + if agent_ctx is not None: + async def _override_get_agent_auth_context() -> AgentAuthContext: + return agent_ctx + + app.dependency_overrides[get_agent_auth_context] = _override_get_agent_auth_context + + if board is not None: + async def _override_get_board_for_actor_read() -> Board: + return board + + app.dependency_overrides[deps_api.get_board_for_actor_read] = _override_get_board_for_actor_read + + return app + + +async def _seed(session: AsyncSession) -> SimpleNamespace: + org = Organization(id=uuid4(), name="Pipeline Org") + user = User( + id=uuid4(), + clerk_user_id="user_123", + email="user@example.com", + name="User", + active_organization_id=org.id, + ) + member = OrganizationMember( + id=uuid4(), + organization_id=org.id, + user_id=user.id, + role="member", + all_boards_read=False, + all_boards_write=False, + ) + connection = ForgejoConnection( + id=uuid4(), + organization_id=org.id, + name="Dream Forgejo", + base_url="https://forgejo.example.local", + token="temp-token", + token_last_eight="p-token", + ) + repository = ForgejoRepository( + id=uuid4(), + organization_id=org.id, + connection_id=connection.id, + owner="openclaw", + repo="pipeline", + display_name="Pipeline", + ) + board = Board( + id=uuid4(), + organization_id=org.id, + name="Pipeline Board", + slug="pipeline-board", + ) + board_access = OrganizationBoardAccess( + id=uuid4(), + organization_member_id=member.id, + board_id=board.id, + can_read=True, + can_write=True, + ) + link = BoardRepositoryLink( + id=uuid4(), + organization_id=org.id, + board_id=board.id, + repository_id=repository.id, + ) + issue = ForgejoIssue( + id=uuid4(), + organization_id=org.id, + repository_id=repository.id, + forgejo_issue_number=42, + title="Cached issue", + body_preview="Cached body", + state="open", + is_pull_request=False, + labels=[], + assignees=[], + author="kaspa", + html_url="https://forgejo.example.local/openclaw/pipeline/issues/42", + forgejo_created_at=datetime(2026, 5, 19, 12, 0, 0), + forgejo_updated_at=datetime(2026, 5, 19, 12, 5, 0), + ) + gateway = Gateway( + id=uuid4(), + organization_id=org.id, + name="Gateway", + url="https://gateway.example.local", + workspace_root="/workspace", + ) + lead_agent = Agent( + id=uuid4(), + board_id=board.id, + gateway_id=gateway.id, + name="Lead Agent", + is_board_lead=True, + ) + worker_agent = Agent( + id=uuid4(), + board_id=board.id, + gateway_id=gateway.id, + name="Worker Agent", + is_board_lead=False, + ) + + session.add(org) + session.add(user) + session.add(member) + session.add(connection) + session.add(repository) + session.add(board) + session.add(board_access) + session.add(link) + session.add(issue) + session.add(gateway) + session.add(lead_agent) + session.add(worker_agent) + await session.commit() + return SimpleNamespace( + org=org, + user=user, + member=member, + board=board, + issue=issue, + lead_agent=lead_agent, + worker_agent=worker_agent, + ) + + +class _StubForgejoClient: + def __init__(self, *, should_fail: bool) -> None: + self._should_fail = should_fail + + async def __aenter__(self) -> _StubForgejoClient: + return self + + async def __aexit__(self, *_args: object) -> None: + return None + + async def close_issue(self, **_kwargs: object) -> dict[str, object]: + if self._should_fail: + raise RuntimeError("forgejo down") + return {"state": "closed"} + + +def _patch_forgejo_client( + monkeypatch: pytest.MonkeyPatch, + *, + should_fail: bool, +) -> None: + def _factory(_connection: object) -> _StubForgejoClient: + return _StubForgejoClient(should_fail=should_fail) + + monkeypatch.setattr(forgejo_issue_close, "get_forgejo_client", _factory) + + +@pytest.mark.asyncio +async def test_human_writer_can_close_linked_issue_and_records_activity( + monkeypatch: pytest.MonkeyPatch, +) -> None: + engine = await _make_engine() + session_maker = async_sessionmaker(engine, class_=AsyncSession, expire_on_commit=False) + + try: + async with session_maker() as session: + seeded = await _seed(session) + + _patch_forgejo_client(monkeypatch, should_fail=False) + org_ctx = OrganizationContext(organization=seeded.org, member=seeded.member) + auth_ctx = AuthContext(actor_type="user", user=seeded.user) + app = _build_test_app(session_maker, org_ctx=org_ctx, auth_ctx=auth_ctx) + + async with AsyncClient( + transport=ASGITransport(app=app), + base_url="http://testserver", + ) as client: + response = await client.post(f"/api/v1/forgejo/issues/{seeded.issue.id}/close") + + assert response.status_code == 200 + payload = response.json() + assert payload["success"] is True + assert payload["state"] == "closed" + assert payload["forgejo_issue_number"] == 42 + + async with session_maker() as session: + stored_issue = await session.get(ForgejoIssue, seeded.issue.id) + assert stored_issue is not None + assert stored_issue.state == "closed" + assert stored_issue.forgejo_closed_at is not None + assert stored_issue.last_synced_at is not None + + events = (await session.exec(select(ActivityEvent))).all() + assert len(events) == 1 + event = events[0] + assert event.event_type == "forgejo.issue.closed" + assert event.board_id == seeded.board.id + assert event.agent_id is None + assert event.message is not None + assert f"user {seeded.user.id}" in event.message + assert "openclaw/pipeline#42" in event.message + finally: + await engine.dispose() + + +@pytest.mark.asyncio +async def test_board_lead_agent_can_close_linked_issue_and_records_activity( + monkeypatch: pytest.MonkeyPatch, +) -> None: + engine = await _make_engine() + session_maker = async_sessionmaker(engine, class_=AsyncSession, expire_on_commit=False) + + try: + async with session_maker() as session: + seeded = await _seed(session) + + _patch_forgejo_client(monkeypatch, should_fail=False) + agent_ctx = AgentAuthContext(actor_type="agent", agent=seeded.lead_agent) + app = _build_test_app( + session_maker, + agent_ctx=agent_ctx, + board=seeded.board, + ) + + async with AsyncClient( + transport=ASGITransport(app=app), + base_url="http://testserver", + ) as client: + response = await client.post( + f"/api/v1/agent/boards/{seeded.board.id}/git/issues/{seeded.issue.id}/close" + ) + + assert response.status_code == 200 + payload = response.json() + assert payload["success"] is True + assert payload["state"] == "closed" + + async with session_maker() as session: + stored_issue = await session.get(ForgejoIssue, seeded.issue.id) + assert stored_issue is not None + assert stored_issue.state == "closed" + + events = (await session.exec(select(ActivityEvent))).all() + assert len(events) == 1 + event = events[0] + assert event.event_type == "forgejo.issue.closed" + assert event.board_id == seeded.board.id + assert event.agent_id == seeded.lead_agent.id + assert event.message is not None + assert f"agent {seeded.lead_agent.id}" in event.message + assert "openclaw/pipeline#42" in event.message + finally: + await engine.dispose() + + +@pytest.mark.asyncio +async def test_worker_agent_cannot_close_issue( + monkeypatch: pytest.MonkeyPatch, +) -> None: + engine = await _make_engine() + session_maker = async_sessionmaker(engine, class_=AsyncSession, expire_on_commit=False) + + try: + async with session_maker() as session: + seeded = await _seed(session) + + _patch_forgejo_client(monkeypatch, should_fail=False) + worker_ctx = AgentAuthContext(actor_type="agent", agent=seeded.worker_agent) + app = _build_test_app( + session_maker, + agent_ctx=worker_ctx, + board=seeded.board, + ) + + async with AsyncClient( + transport=ASGITransport(app=app), + base_url="http://testserver", + ) as client: + response = await client.post( + f"/api/v1/agent/boards/{seeded.board.id}/git/issues/{seeded.issue.id}/close" + ) + + assert response.status_code == 403 + assert response.json()["detail"] == "Only board leads can close issues" + + async with session_maker() as session: + stored_issue = await session.get(ForgejoIssue, seeded.issue.id) + assert stored_issue is not None + assert stored_issue.state == "open" + events = (await session.exec(select(ActivityEvent))).all() + assert events == [] + finally: + await engine.dispose() + + +@pytest.mark.asyncio +async def test_forgejo_close_failure_does_not_update_local_issue( + monkeypatch: pytest.MonkeyPatch, +) -> None: + engine = await _make_engine() + session_maker = async_sessionmaker(engine, class_=AsyncSession, expire_on_commit=False) + + try: + async with session_maker() as session: + seeded = await _seed(session) + + _patch_forgejo_client(monkeypatch, should_fail=True) + org_ctx = OrganizationContext(organization=seeded.org, member=seeded.member) + auth_ctx = AuthContext(actor_type="user", user=seeded.user) + app = _build_test_app(session_maker, org_ctx=org_ctx, auth_ctx=auth_ctx) + + async with AsyncClient( + transport=ASGITransport(app=app), + base_url="http://testserver", + ) as client: + response = await client.post(f"/api/v1/forgejo/issues/{seeded.issue.id}/close") + + assert response.status_code == 502 + assert "Failed to close issue on Forgejo" in response.json()["detail"] + + async with session_maker() as session: + stored_issue = await session.get(ForgejoIssue, seeded.issue.id) + assert stored_issue is not None + assert stored_issue.state == "open" + assert stored_issue.forgejo_closed_at is None + events = (await session.exec(select(ActivityEvent))).all() + assert events == [] + finally: + await engine.dispose()