From 0a15ad9522505e556045b97495dad125c09cedb2 Mon Sep 17 00:00:00 2001 From: pyr0ball Date: Sun, 5 Apr 2026 17:31:02 -0700 Subject: [PATCH 1/2] =?UTF-8?q?feat:=20add=20circuitforge=5Fcore.api.feedb?= =?UTF-8?q?ack=20=E2=80=94=20shared=20feedback=20router=20factory=20(close?= =?UTF-8?q?s=20#23)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds make_feedback_router(repo, product, demo_mode_fn) which returns a FastAPI APIRouter with GET /status and POST / endpoints. Handles Forgejo label creation/reuse, issue body assembly (including repro steps for bugs), demo mode gating, and FORGEJO_API_TOKEN presence checks. 12 tests covering all status/submit paths, mock Forgejo interaction, and body content assertions. Also adds fastapi>=0.110 and httpx>=0.27 to [dev] optional deps. --- circuitforge_core/api/__init__.py | 0 circuitforge_core/api/feedback.py | 174 +++++++++++++++++ pyproject.toml | 2 + tests/test_api/__init__.py | 0 tests/test_api/test_feedback.py | 298 ++++++++++++++++++++++++++++++ 5 files changed, 474 insertions(+) create mode 100644 circuitforge_core/api/__init__.py create mode 100644 circuitforge_core/api/feedback.py create mode 100644 tests/test_api/__init__.py create mode 100644 tests/test_api/test_feedback.py diff --git a/circuitforge_core/api/__init__.py b/circuitforge_core/api/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/circuitforge_core/api/feedback.py b/circuitforge_core/api/feedback.py new file mode 100644 index 0000000..5a1df56 --- /dev/null +++ b/circuitforge_core/api/feedback.py @@ -0,0 +1,174 @@ +""" +Shared feedback router — creates Forgejo issues from in-app beta feedback. +Products include this with make_feedback_router(repo=..., product=...). +""" +from __future__ import annotations + +import os +import platform +import subprocess +from collections.abc import Callable +from datetime import datetime, timezone +from pathlib import Path +from typing import Literal + +import requests +from fastapi import APIRouter, HTTPException +from pydantic import BaseModel + +_LABEL_COLORS: dict[str, str] = { + "beta-feedback": "#0075ca", + "needs-triage": "#e4e669", + "bug": "#d73a4a", + "feature-request": "#a2eeef", + "question": "#d876e3", +} + +_TYPE_LABEL_MAP: dict[str, str] = {"bug": "bug", "feature": "feature-request"} +_TYPE_DISPLAY: dict[str, str] = { + "bug": "🐛 Bug", + "feature": "✨ Feature Request", + "other": "💬 Other", +} + + +class FeedbackRequest(BaseModel): + title: str + description: str + type: Literal["bug", "feature", "other"] = "other" + repro: str = "" + tab: str = "unknown" + submitter: str = "" + + +class FeedbackResponse(BaseModel): + issue_number: int + issue_url: str + + +def _forgejo_headers() -> dict[str, str]: + token = os.environ.get("FORGEJO_API_TOKEN", "") + return {"Authorization": f"token {token}", "Content-Type": "application/json"} + + +def _ensure_labels(label_names: list[str], base: str, repo: str) -> list[int]: + headers = _forgejo_headers() + resp = requests.get(f"{base}/repos/{repo}/labels", headers=headers, timeout=10) + existing = {lb["name"]: lb["id"] for lb in resp.json()} if resp.ok else {} + ids: list[int] = [] + for name in label_names: + if name in existing: + ids.append(existing[name]) + else: + r = requests.post( + f"{base}/repos/{repo}/labels", + headers=headers, + json={"name": name, "color": _LABEL_COLORS.get(name, "#ededed")}, + timeout=10, + ) + if r.ok: + ids.append(r.json()["id"]) + return ids + + +def _collect_context(tab: str, product: str) -> dict[str, str]: + try: + version = subprocess.check_output( + ["git", "describe", "--tags", "--always"], + cwd=Path.cwd(), + text=True, + timeout=5, + ).strip() + except Exception: + version = "dev" + return { + "product": product, + "tab": tab, + "version": version, + "platform": platform.platform(), + "timestamp": datetime.now(timezone.utc).isoformat().replace("+00:00", "Z"), + } + + +def _build_issue_body(payload: FeedbackRequest, context: dict[str, str]) -> str: + lines: list[str] = [ + f"## {_TYPE_DISPLAY.get(payload.type, '💬 Other')}", + "", + payload.description, + "", + ] + if payload.type == "bug" and payload.repro: + lines += ["### Reproduction Steps", "", payload.repro, ""] + lines += ["### Context", ""] + for k, v in context.items(): + lines.append(f"- **{k}:** {v}") + lines.append("") + if payload.submitter: + lines += ["---", f"*Submitted by: {payload.submitter}*"] + return "\n".join(lines) + + +def make_feedback_router( + repo: str, + product: str, + demo_mode_fn: Callable[[], bool] | None = None, +) -> APIRouter: + """Return a configured feedback APIRouter for the given Forgejo repo and product. + + Args: + repo: Forgejo repo slug, e.g. "Circuit-Forge/kiwi". + product: Product name injected into issue context, e.g. "kiwi". + demo_mode_fn: Optional callable returning True when in demo mode. + If None, reads the DEMO_MODE environment variable. + """ + + def _is_demo() -> bool: + if demo_mode_fn is not None: + return demo_mode_fn() + return os.environ.get("DEMO_MODE", "").lower() in ("1", "true", "yes") + + router = APIRouter() + + @router.get("/status") + def feedback_status() -> dict: + """Return whether feedback submission is configured on this instance.""" + return {"enabled": bool(os.environ.get("FORGEJO_API_TOKEN")) and not _is_demo()} + + @router.post("", response_model=FeedbackResponse) + def submit_feedback(payload: FeedbackRequest) -> FeedbackResponse: + """File a Forgejo issue from in-app feedback.""" + token = os.environ.get("FORGEJO_API_TOKEN", "") + if not token: + raise HTTPException( + status_code=503, + detail="Feedback disabled: FORGEJO_API_TOKEN not configured.", + ) + if _is_demo(): + raise HTTPException(status_code=403, detail="Feedback disabled in demo mode.") + + base = os.environ.get( + "FORGEJO_API_URL", "https://git.opensourcesolarpunk.com/api/v1" + ) + context = _collect_context(payload.tab, product) + body = _build_issue_body(payload, context) + labels = [ + "beta-feedback", + "needs-triage", + _TYPE_LABEL_MAP.get(payload.type, "question"), + ] + label_ids = _ensure_labels(labels, base, repo) + + resp = requests.post( + f"{base}/repos/{repo}/issues", + headers=_forgejo_headers(), + json={"title": payload.title, "body": body, "labels": label_ids}, + timeout=15, + ) + if not resp.ok: + raise HTTPException( + status_code=502, detail=f"Forgejo error: {resp.text[:200]}" + ) + data = resp.json() + return FeedbackResponse(issue_number=data["number"], issue_url=data["html_url"]) + + return router diff --git a/pyproject.toml b/pyproject.toml index eecccd8..cb0c0fe 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,6 +22,8 @@ dev = [ "circuitforge-core[manage]", "pytest>=8.0", "pytest-asyncio>=0.23", + "fastapi>=0.110", + "httpx>=0.27", ] [project.scripts] diff --git a/tests/test_api/__init__.py b/tests/test_api/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_api/test_feedback.py b/tests/test_api/test_feedback.py new file mode 100644 index 0000000..9b9e545 --- /dev/null +++ b/tests/test_api/test_feedback.py @@ -0,0 +1,298 @@ +"""Tests for circuitforge_core.api.feedback — shared feedback router factory.""" +from __future__ import annotations + +import json +from unittest.mock import MagicMock, patch + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from circuitforge_core.api.feedback import make_feedback_router + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _make_client( + demo_mode_fn=None, + repo: str = "Circuit-Forge/test", + product: str = "test", +) -> TestClient: + app = FastAPI() + router = make_feedback_router(repo=repo, product=product, demo_mode_fn=demo_mode_fn) + app.include_router(router, prefix="/feedback") + return TestClient(app) + + +def _mock_forgejo_get(label_list: list[dict] | None = None): + """Return a mock for requests.get that returns an empty label list.""" + mock = MagicMock() + mock.ok = True + mock.json.return_value = label_list or [] + return mock + + +def _mock_forgejo_post_issue(number: int = 42, url: str = "https://example.com/issues/42"): + """Return a mock for requests.post that simulates a successful issue creation.""" + mock = MagicMock() + mock.ok = True + mock.json.return_value = {"number": number, "html_url": url} + return mock + + +_VALID_PAYLOAD = { + "title": "Something broke", + "description": "It stopped working after the update.", + "type": "bug", + "repro": "1. Open app\n2. Click submit\n3. See error", + "tab": "dashboard", + "submitter": "alan@example.com", +} + +# --------------------------------------------------------------------------- +# GET /feedback/status +# --------------------------------------------------------------------------- + +def test_status_no_token_returns_disabled(monkeypatch): + """GET /status returns enabled=False when FORGEJO_API_TOKEN is not set.""" + monkeypatch.delenv("FORGEJO_API_TOKEN", raising=False) + monkeypatch.delenv("DEMO_MODE", raising=False) + client = _make_client() + resp = client.get("/feedback/status") + assert resp.status_code == 200 + assert resp.json() == {"enabled": False} + + +def test_status_with_token_returns_enabled(monkeypatch): + """GET /status returns enabled=True when token is set and not in demo mode.""" + monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc") + monkeypatch.delenv("DEMO_MODE", raising=False) + client = _make_client() + resp = client.get("/feedback/status") + assert resp.status_code == 200 + assert resp.json() == {"enabled": True} + + +def test_status_demo_mode_env_returns_disabled(monkeypatch): + """GET /status returns enabled=False when DEMO_MODE=1 even with a token.""" + monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc") + monkeypatch.setenv("DEMO_MODE", "1") + client = _make_client() + resp = client.get("/feedback/status") + assert resp.status_code == 200 + assert resp.json() == {"enabled": False} + + +def test_status_demo_mode_fn_returns_disabled(monkeypatch): + """GET /status returns enabled=False when demo_mode_fn() returns True.""" + monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc") + monkeypatch.delenv("DEMO_MODE", raising=False) + client = _make_client(demo_mode_fn=lambda: True) + resp = client.get("/feedback/status") + assert resp.status_code == 200 + assert resp.json() == {"enabled": False} + + +# --------------------------------------------------------------------------- +# POST /feedback +# --------------------------------------------------------------------------- + +def test_post_no_token_returns_503(monkeypatch): + """POST / returns 503 when FORGEJO_API_TOKEN is not configured.""" + monkeypatch.delenv("FORGEJO_API_TOKEN", raising=False) + monkeypatch.delenv("DEMO_MODE", raising=False) + client = _make_client() + resp = client.post("/feedback", json=_VALID_PAYLOAD) + assert resp.status_code == 503 + assert "FORGEJO_API_TOKEN" in resp.json()["detail"] + + +def test_post_demo_mode_fn_returns_403(monkeypatch): + """POST / returns 403 when demo_mode_fn returns True.""" + monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc") + monkeypatch.delenv("DEMO_MODE", raising=False) + client = _make_client(demo_mode_fn=lambda: True) + resp = client.post("/feedback", json=_VALID_PAYLOAD) + assert resp.status_code == 403 + assert "demo" in resp.json()["detail"].lower() + + +def test_post_success_returns_issue_number_and_url(monkeypatch): + """POST / returns issue_number and issue_url on success.""" + monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc") + monkeypatch.delenv("DEMO_MODE", raising=False) + monkeypatch.setenv("FORGEJO_API_URL", "https://forgejo.test/api/v1") + + mock_get = _mock_forgejo_get() + mock_post_label = MagicMock(ok=True) + mock_post_label.json.return_value = {"id": 99, "name": "beta-feedback"} + mock_post_issue = _mock_forgejo_post_issue(number=7, url="https://forgejo.test/Circuit-Forge/test/issues/7") + + # requests.post is called multiple times: once per new label, then once for the issue. + # We use side_effect to distinguish label creation calls from the issue creation call. + post_calls = [] + + def post_side_effect(url, **kwargs): + post_calls.append(url) + if "/labels" in url: + return mock_post_label + return mock_post_issue + + client = _make_client() + with patch("circuitforge_core.api.feedback.requests.get", return_value=mock_get), \ + patch("circuitforge_core.api.feedback.requests.post", side_effect=post_side_effect): + resp = client.post("/feedback", json=_VALID_PAYLOAD) + + assert resp.status_code == 200 + data = resp.json() + assert data["issue_number"] == 7 + assert data["issue_url"] == "https://forgejo.test/Circuit-Forge/test/issues/7" + + +def test_post_forgejo_error_returns_502(monkeypatch): + """POST / returns 502 when Forgejo returns a non-ok response for issue creation.""" + monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc") + monkeypatch.delenv("DEMO_MODE", raising=False) + monkeypatch.setenv("FORGEJO_API_URL", "https://forgejo.test/api/v1") + + mock_get = _mock_forgejo_get() + mock_issue_error = MagicMock(ok=False) + mock_issue_error.text = "Internal Server Error" + + def post_side_effect(url, **kwargs): + if "/labels" in url: + m = MagicMock(ok=True) + m.json.return_value = {"id": 1, "name": "beta-feedback"} + return m + return mock_issue_error + + client = _make_client() + with patch("circuitforge_core.api.feedback.requests.get", return_value=mock_get), \ + patch("circuitforge_core.api.feedback.requests.post", side_effect=post_side_effect): + resp = client.post("/feedback", json=_VALID_PAYLOAD) + + assert resp.status_code == 502 + assert "Forgejo error" in resp.json()["detail"] + + +def test_post_product_name_appears_in_issue_body(monkeypatch): + """The product name passed to make_feedback_router appears in the issue body context.""" + monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc") + monkeypatch.delenv("DEMO_MODE", raising=False) + monkeypatch.setenv("FORGEJO_API_URL", "https://forgejo.test/api/v1") + + captured_body: list[str] = [] + mock_get = _mock_forgejo_get() + + def post_side_effect(url, **kwargs): + if "/labels" in url: + m = MagicMock(ok=True) + m.json.return_value = {"id": 1, "name": "beta-feedback"} + return m + # Capture the body sent for the issue creation call + captured_body.append(kwargs.get("json", {}).get("body", "")) + m = MagicMock(ok=True) + m.json.return_value = {"number": 1, "html_url": "https://forgejo.test/issues/1"} + return m + + client = _make_client(product="kiwi") + with patch("circuitforge_core.api.feedback.requests.get", return_value=mock_get), \ + patch("circuitforge_core.api.feedback.requests.post", side_effect=post_side_effect): + resp = client.post( + "/feedback", + json={ + "title": "Pantry bug", + "description": "Items disappear.", + "type": "bug", + "tab": "pantry", + }, + ) + + assert resp.status_code == 200 + assert captured_body, "No issue body was captured" + assert "kiwi" in captured_body[0], f"Product name not found in body: {captured_body[0]}" + + +def test_post_bug_with_repro_includes_repro_section(monkeypatch): + """A bug report with a repro string includes the Reproduction Steps section in the body.""" + monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc") + monkeypatch.delenv("DEMO_MODE", raising=False) + monkeypatch.setenv("FORGEJO_API_URL", "https://forgejo.test/api/v1") + + captured_body: list[str] = [] + mock_get = _mock_forgejo_get() + + def post_side_effect(url, **kwargs): + if "/labels" in url: + m = MagicMock(ok=True) + m.json.return_value = {"id": 1, "name": "bug"} + return m + captured_body.append(kwargs.get("json", {}).get("body", "")) + m = MagicMock(ok=True) + m.json.return_value = {"number": 2, "html_url": "https://forgejo.test/issues/2"} + return m + + repro_text = "1. Open the app\n2. Click the button\n3. Observe crash" + client = _make_client() + with patch("circuitforge_core.api.feedback.requests.get", return_value=mock_get), \ + patch("circuitforge_core.api.feedback.requests.post", side_effect=post_side_effect): + resp = client.post( + "/feedback", + json={ + "title": "App crashes", + "description": "The app crashes on button click.", + "type": "bug", + "repro": repro_text, + "tab": "home", + }, + ) + + assert resp.status_code == 200 + assert captured_body, "No issue body was captured" + body = captured_body[0] + assert "Reproduction Steps" in body, f"'Reproduction Steps' not found in body: {body}" + assert repro_text in body, f"Repro text not found in body: {body}" + + +def test_status_demo_mode_env_true_string(monkeypatch): + """GET /status treats DEMO_MODE=true as demo mode.""" + monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc") + monkeypatch.setenv("DEMO_MODE", "true") + client = _make_client() + resp = client.get("/feedback/status") + assert resp.status_code == 200 + assert resp.json() == {"enabled": False} + + +def test_post_existing_labels_reused(monkeypatch): + """When labels already exist on Forgejo, their IDs are reused (no POST to /labels).""" + monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token-abc") + monkeypatch.delenv("DEMO_MODE", raising=False) + monkeypatch.setenv("FORGEJO_API_URL", "https://forgejo.test/api/v1") + + existing_labels = [ + {"name": "beta-feedback", "id": 10}, + {"name": "needs-triage", "id": 11}, + {"name": "bug", "id": 12}, + ] + mock_get = _mock_forgejo_get(existing_labels) + label_post_calls: list[str] = [] + + def post_side_effect(url, **kwargs): + if "/labels" in url: + label_post_calls.append(url) + m = MagicMock(ok=True) + m.json.return_value = {"id": 99, "name": "new-label"} + return m + m = MagicMock(ok=True) + m.json.return_value = {"number": 5, "html_url": "https://forgejo.test/issues/5"} + return m + + client = _make_client() + with patch("circuitforge_core.api.feedback.requests.get", return_value=mock_get), \ + patch("circuitforge_core.api.feedback.requests.post", side_effect=post_side_effect): + resp = client.post("/feedback", json=_VALID_PAYLOAD) + + assert resp.status_code == 200 + assert label_post_calls == [], "Should not POST to /labels when all labels already exist" From f0a9ec5c3747de9acfe3fbaf00a8e8a85660b464 Mon Sep 17 00:00:00 2001 From: pyr0ball Date: Sun, 5 Apr 2026 17:36:52 -0700 Subject: [PATCH 2/2] fix: raise 502 on label creation failure; narrow subprocess exception scope --- circuitforge_core/api/feedback.py | 7 ++++++- tests/test_api/test_feedback.py | 33 +++++++++++++++++++++++++++++-- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/circuitforge_core/api/feedback.py b/circuitforge_core/api/feedback.py index 5a1df56..495678d 100644 --- a/circuitforge_core/api/feedback.py +++ b/circuitforge_core/api/feedback.py @@ -68,6 +68,11 @@ def _ensure_labels(label_names: list[str], base: str, repo: str) -> list[int]: ) if r.ok: ids.append(r.json()["id"]) + else: + raise HTTPException( + status_code=502, + detail=f"Failed to create label '{name}': {r.text[:200]}", + ) return ids @@ -79,7 +84,7 @@ def _collect_context(tab: str, product: str) -> dict[str, str]: text=True, timeout=5, ).strip() - except Exception: + except (subprocess.SubprocessError, OSError): version = "dev" return { "product": product, diff --git a/tests/test_api/test_feedback.py b/tests/test_api/test_feedback.py index 9b9e545..3e03512 100644 --- a/tests/test_api/test_feedback.py +++ b/tests/test_api/test_feedback.py @@ -131,12 +131,12 @@ def test_post_success_returns_issue_number_and_url(monkeypatch): # requests.post is called multiple times: once per new label, then once for the issue. # We use side_effect to distinguish label creation calls from the issue creation call. - post_calls = [] + issue_call_kwargs: list[dict] = [] def post_side_effect(url, **kwargs): - post_calls.append(url) if "/labels" in url: return mock_post_label + issue_call_kwargs.append(kwargs) return mock_post_issue client = _make_client() @@ -149,6 +149,35 @@ def test_post_success_returns_issue_number_and_url(monkeypatch): assert data["issue_number"] == 7 assert data["issue_url"] == "https://forgejo.test/Circuit-Forge/test/issues/7" + # Verify label IDs were forwarded to the issue creation call. + # mock_post_label returns id=99 for each of the 3 new labels → [99, 99, 99] + assert issue_call_kwargs, "Issue creation call was not made" + assert issue_call_kwargs[0]["json"]["labels"] == [99, 99, 99] + + +def test_post_returns_502_on_label_creation_failure(monkeypatch): + """POST / returns 502 when Forgejo label creation fails.""" + monkeypatch.setenv("FORGEJO_API_TOKEN", "test-token") + monkeypatch.delenv("DEMO_MODE", raising=False) + monkeypatch.setenv("FORGEJO_API_URL", "https://forgejo.test/api/v1") + + label_list_response = MagicMock() + label_list_response.ok = True + label_list_response.json.return_value = [] # no existing labels + + label_create_response = MagicMock() + label_create_response.ok = False + label_create_response.text = "forbidden" + + client = _make_client() + with patch("circuitforge_core.api.feedback.requests.get", return_value=label_list_response), \ + patch("circuitforge_core.api.feedback.requests.post", return_value=label_create_response): + res = client.post("/feedback", json={ + "title": "Test", "description": "desc", "type": "other", + }) + assert res.status_code == 502 + assert "beta-feedback" in res.json()["detail"] + def test_post_forgejo_error_returns_502(monkeypatch): """POST / returns 502 when Forgejo returns a non-ok response for issue creation."""