From d45fd42c7fcde297c72c11e4b47dd50591058a28 Mon Sep 17 00:00:00 2001 From: null Date: Wed, 20 May 2026 01:35:45 -0500 Subject: [PATCH] bug: git --- backend/app/api/forgejo_metrics.py | 36 ++-- backend/tests/test_forgejo_metrics_api.py | 191 ++++++++++++++++++ frontend/src/app/dashboard/page.tsx | 12 +- .../git/ForgejoIssueMetricCards.tsx | 4 +- 4 files changed, 226 insertions(+), 17 deletions(-) create mode 100644 backend/tests/test_forgejo_metrics_api.py diff --git a/backend/app/api/forgejo_metrics.py b/backend/app/api/forgejo_metrics.py index ace4468..0a8845a 100644 --- a/backend/app/api/forgejo_metrics.py +++ b/backend/app/api/forgejo_metrics.py @@ -4,21 +4,19 @@ from __future__ import annotations from datetime import datetime, timedelta, timezone from typing import TYPE_CHECKING +from uuid import UUID from fastapi import APIRouter, Depends, HTTPException, Query, status -from sqlmodel import select, func from sqlalchemy import and_ +from sqlmodel import func, select from app.api.deps import ORG_MEMBER_DEP, OrganizationContext -from app.core.agent_auth import get_agent_auth_context, AgentAuthContext from app.db.session import get_session from app.models.board_repository_links import BoardRepositoryLink from app.models.forgejo_issues import ForgejoIssue from app.models.forgejo_repositories import ForgejoRepository from app.schemas.metrics import MetricsResponse -from app.core.agent_auth import get_agent_auth_context, AgentAuthContext - if TYPE_CHECKING: from sqlmodel.ext.asyncio.session import AsyncSession @@ -68,15 +66,15 @@ SESSION_DEP = Depends(get_session) }, ) async def get_forgejo_metrics( - organization_id: str | None = Query( + organization_id: UUID | None = Query( None, description="Filter by organization ID", ), - board_id: str | None = Query( + board_id: UUID | None = Query( None, description="Filter by board ID (via linked repositories)", ), - repository_id: str | None = Query( + repository_id: UUID | None = Query( None, description="Filter by specific repository ID", ), @@ -93,18 +91,29 @@ async def get_forgejo_metrics( Empty scope (no filters) returns zeroed metrics. """ # Determine scope + if organization_id and organization_id != ctx.organization.id: + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) + if repository_id: # Single repository - repo_ids = [repository_id] if board_id or organization_id: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail="Cannot combine repository_id with board_id or organization_id", ) + repo_statement = select(ForgejoRepository).where( + ForgejoRepository.id == repository_id, + ForgejoRepository.organization_id == ctx.organization.id, + ) + repo = (await session.exec(repo_statement)).first() + if repo is None: + return _zeroed_metrics() + repo_ids = [repo.id] elif board_id: # Board-scoped: get linked repositories link_statement = select(BoardRepositoryLink).where( - BoardRepositoryLink.board_id == board_id + BoardRepositoryLink.board_id == board_id, + BoardRepositoryLink.organization_id == ctx.organization.id, ) links = (await session.exec(link_statement)).all() repo_ids = [link.repository_id for link in links] @@ -113,10 +122,10 @@ async def get_forgejo_metrics( elif organization_id: # Organization-scoped: all repositories in org repo_statement = select(ForgejoRepository.id).where( - ForgejoRepository.organization_id == organization_id + ForgejoRepository.organization_id == ctx.organization.id, ) repos = (await session.exec(repo_statement)).all() - repo_ids = [r.id for r in repos] + repo_ids = list(repos) if not repo_ids: return _zeroed_metrics() else: @@ -197,12 +206,13 @@ async def get_forgejo_metrics( ) repo = (await session.exec(repo_statement)).first() if repo: - last_sync_timestamps[repo_id] = ( + repo_key = str(repo_id) + last_sync_timestamps[repo_key] = ( repo.last_sync_at.isoformat() if repo.last_sync_at else "" ) - sync_error_counts[repo_id] = 1 if repo.last_sync_error else 0 + sync_error_counts[repo_key] = 1 if repo.last_sync_error else 0 repositories_synced = len(repo_ids) diff --git a/backend/tests/test_forgejo_metrics_api.py b/backend/tests/test_forgejo_metrics_api.py new file mode 100644 index 0000000..533f63d --- /dev/null +++ b/backend/tests/test_forgejo_metrics_api.py @@ -0,0 +1,191 @@ +# ruff: noqa: INP001 +"""Integration tests for Forgejo metrics scoping.""" + +from __future__ import annotations + +from datetime import datetime +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 +from sqlmodel.ext.asyncio.session import AsyncSession + +from app import models as _models +from app.api.deps import require_org_member +from app.api.forgejo_metrics import router as forgejo_metrics_router +from app.db.session import get_session +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.organization_members import OrganizationMember +from app.models.organizations import Organization +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], + ctx: OrganizationContext, +) -> FastAPI: + app = FastAPI() + api_v1 = APIRouter(prefix="/api/v1") + api_v1.include_router(forgejo_metrics_router) + app.include_router(api_v1) + + async def _override_get_session() -> AsyncSession: + async with session_maker() as session: + yield session + + async def _override_require_org_member() -> OrganizationContext: + return ctx + + app.dependency_overrides[get_session] = _override_get_session + app.dependency_overrides[require_org_member] = _override_require_org_member + return app + + +def _issue( + *, + organization_id, + repository_id, + issue_number: int, +) -> ForgejoIssue: + return ForgejoIssue( + id=uuid4(), + organization_id=organization_id, + repository_id=repository_id, + forgejo_issue_number=issue_number, + title=f"Issue {issue_number}", + body_preview="Cached issue body", + state="open", + is_pull_request=False, + labels=[], + assignees=[], + author="kaspa", + html_url=f"https://forgejo.example.local/openclaw/pipeline/issues/{issue_number}", + forgejo_created_at=datetime(2026, 5, 19, 12, 0, 0), + forgejo_updated_at=datetime(2026, 5, 19, 12, 5, 0), + ) + + +@pytest.mark.asyncio +async def test_forgejo_metrics_are_scoped_to_active_organization() -> None: + engine = await _make_engine() + session_maker = async_sessionmaker(engine, class_=AsyncSession, expire_on_commit=False) + org_a = Organization(id=uuid4(), name="Org A") + org_b = Organization(id=uuid4(), name="Org B") + member = OrganizationMember( + id=uuid4(), + organization_id=org_a.id, + user_id=uuid4(), + role="owner", + ) + app = _build_test_app( + session_maker, + OrganizationContext(organization=org_a, member=member), + ) + + conn_a = ForgejoConnection( + id=uuid4(), + organization_id=org_a.id, + name="A Forgejo", + base_url="https://forgejo-a.example.local", + ) + conn_b = ForgejoConnection( + id=uuid4(), + organization_id=org_b.id, + name="B Forgejo", + base_url="https://forgejo-b.example.local", + ) + repo_a = ForgejoRepository( + id=uuid4(), + organization_id=org_a.id, + connection_id=conn_a.id, + owner="openclaw", + repo="pipeline", + display_name="Pipeline", + ) + repo_b = ForgejoRepository( + id=uuid4(), + organization_id=org_b.id, + connection_id=conn_b.id, + owner="other", + repo="private", + display_name="Private", + ) + board_b = Board( + id=uuid4(), + organization_id=org_b.id, + name="Other Board", + slug="other-board", + ) + link_b = BoardRepositoryLink( + id=uuid4(), + organization_id=org_b.id, + board_id=board_b.id, + repository_id=repo_b.id, + ) + + try: + async with session_maker() as session: + session.add(org_a) + session.add(org_b) + session.add(conn_a) + session.add(conn_b) + session.add(repo_a) + session.add(repo_b) + session.add(board_b) + session.add(link_b) + session.add( + _issue( + organization_id=org_a.id, + repository_id=repo_a.id, + issue_number=1, + ) + ) + session.add( + _issue( + organization_id=org_b.id, + repository_id=repo_b.id, + issue_number=2, + ) + ) + await session.commit() + + async with AsyncClient( + transport=ASGITransport(app=app), + base_url="http://testserver", + ) as client: + own_org = await client.get( + f"/api/v1/forgejo/metrics?organization_id={org_a.id}" + ) + other_org = await client.get( + f"/api/v1/forgejo/metrics?organization_id={org_b.id}" + ) + other_repo = await client.get( + f"/api/v1/forgejo/metrics?repository_id={repo_b.id}" + ) + other_board = await client.get( + f"/api/v1/forgejo/metrics?board_id={board_b.id}" + ) + + assert own_org.status_code == 200 + assert own_org.json()["open_issues"] == 1 + assert other_org.status_code == 403 + assert other_repo.status_code == 200 + assert other_repo.json()["open_issues"] == 0 + assert other_board.status_code == 200 + assert other_board.json()["open_issues"] == 0 + finally: + await engine.dispose() diff --git a/frontend/src/app/dashboard/page.tsx b/frontend/src/app/dashboard/page.tsx index df9f060..3bed933 100644 --- a/frontend/src/app/dashboard/page.tsx +++ b/frontend/src/app/dashboard/page.tsx @@ -230,6 +230,14 @@ const formatPerDay = (total: number, days: number): string => { return `${(total / days).toFixed(1)}/day`; }; +const formatForgejoDashboardError = (error: Error | null): string | null => { + if (!error) return null; + if (error.message === "Failed to fetch") { + return "Pipeline could not load Git Project data. Check that the frontend can reach the backend API."; + } + return error.message; +}; + const toSessionSummaries = ( sessions: unknown[] | null | undefined, mainSession: unknown, @@ -813,8 +821,8 @@ export default function DashboardPage() { const activityFeedHref = "/activity"; const forgejoIssueMetrics = forgejoMetricsQuery.data ?? null; const forgejoIssueMetricsError = - forgejoRepositoriesQuery.error?.message ?? - forgejoMetricsQuery.error?.message ?? + formatForgejoDashboardError(forgejoRepositoriesQuery.error) ?? + formatForgejoDashboardError(forgejoMetricsQuery.error) ?? null; const forgejoIssueMetricsLoading = forgejoRepositoriesQuery.isLoading || forgejoMetricsQuery.isLoading; diff --git a/frontend/src/components/git/ForgejoIssueMetricCards.tsx b/frontend/src/components/git/ForgejoIssueMetricCards.tsx index 4653602..0f0d4e2 100644 --- a/frontend/src/components/git/ForgejoIssueMetricCards.tsx +++ b/frontend/src/components/git/ForgejoIssueMetricCards.tsx @@ -271,12 +271,12 @@ export function ForgejoIssueMetricCards({ ))} - {!isLoading && repositories.length === 0 ? ( + {!isLoading && !error && repositories.length === 0 ? (
No Git Project repositories are configured yet. Metrics will populate after repositories are added and synced.
- ) : !isLoading && repositoriesSynced === 0 ? ( + ) : !isLoading && !error && repositoriesSynced === 0 ? (
Git Project repositories are configured, but Pipeline has not synced issue metrics yet.