From f0a9ec5c3747de9acfe3fbaf00a8e8a85660b464 Mon Sep 17 00:00:00 2001 From: pyr0ball Date: Sun, 5 Apr 2026 17:36:52 -0700 Subject: [PATCH] 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."""