From 52d6ac419a0f470b8e53d7502acbdc7ae6d5e7d8 Mon Sep 17 00:00:00 2001 From: maziggy Date: Wed, 13 May 2026 11:38:36 +0200 Subject: [PATCH] feat(support): record slicer CLI versions; harden sidecar update docs Issue #1312 follow-up. Investigation traced the "Name cannot be empty" report to a sidecar image pre-dating the /profiles/bundle endpoint addition. Two changes so the next occurrence is self-diagnosable from the support bundle without a manual curl. Backend: new _fetch_slicer_health(url) helper does a 2s GET on /health, walks every non-dataPath key under checks looking for a version field (the wrapper labels both sidecars as checks.orcaslicer regardless of which CLI is bundled). _collect_slicer_api_info now exposes bambu_studio_version and orcaslicer_version. Strips trailing slash before appending /health to avoid double-slash 404s. Docs: bambuddy-wiki/docs/features/slicer-api.md gains a Quick Start callout that branch-built sidecars don't auto-update, a corrected /health troubleshooting entry (both "unknown" version and "orcaslicer" field name on bambu-studio-api are cosmetic wrapper bugs, not stale- image indicators), a new "Name cannot be empty" troubleshooting entry, and an Updating section that requires --no-cache --pull together (BuildKit caches the git context separately from layers, so --no-cache alone silently reuses the old checkout). --- CHANGELOG.md | 6 + backend/app/api/routes/support.py | 51 +++++- backend/tests/unit/test_support_helpers.py | 179 +++++++++++++++++++-- 3 files changed, 220 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd0c40e01..8e8c24f16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,12 @@ All notable changes to Bambuddy will be documented in this file. - **Copy spool — duplicate any spool's settings into a fresh inventory row in two clicks** ([#1234](https://github.com/maziggy/bambuddy/issues/1234), [PR #1246](https://github.com/maziggy/bambuddy/pull/1246) by @MiguelAngelLV) — Adds a copy button (`Copy` icon) next to the existing edit button on every spool in the inventory page across all three views (table row, card, grouped table inner row). Clicking it opens the existing `SpoolFormModal` pre-filled with every field from the source spool — material, brand, color, slicer preset, label/core/cost, K-profiles, all of it — except `weight_used` which is reset to 0 (since the new spool starts full) and the RFID identity fields (`tag_uid`, `tray_uuid`, `tag_type`, `data_origin`) which aren't part of the form payload anyway, so the new spool is its own physical roll. Save calls `api.createSpool` (or `api.createSpoolmanInventorySpool` in Spoolman mode — both inherit the dispatch routing for free). Closes the long-running gap where users with many near-identical spools (e.g. five 1 kg PETG-CF rolls bought in a single order) had to re-enter every field from scratch on each one. **Implementation shape:** `SpoolFormModalProps.mode: 'create' | 'edit' | 'copy'` (exported as `SpoolFormMode`) replaces the previous `isEditing = !!spool` heuristic — every existing call site in `InventoryPage.tsx` was updated to pass the explicit mode, and the modal's title / submit-button label / weight-reset gate / submit-route branching all key on `mode` directly. The `onCopy` callback is optional on `SpoolCard`, `SpoolTableRow`, and `SpoolTableGroup` (matches the existing `onPrintLabel?` pattern), so the button is conditionally rendered and other consumers of those subcomponents don't get a copy affordance forced on them. Card-view and table-row buttons stop click propagation so clicking copy doesn't also fire the parent row's edit handler. **Quick Add interaction:** the Quick Add toggle is gated `mode === 'create'` (was `!isEditing`), so it stays out of copy mode — otherwise a user could enable Quick Add and bump quantity to N under the singular "Copy Spool" title and silently bulk-create N copies via `bulkCreateMutation`. **i18n:** new `inventory.copySpool` key across all 8 locales (en + de translated, fr/it/ja/pt-BR/zh-CN/zh-TW seeded with English fallback per project flow). **Tests:** 3 new in `SpoolFormModal.test.tsx` (`SpoolFormModal copy mode` describe block — title shows "Copy Spool", save calls `createSpool` not `updateSpool`, `weight_used` reset to 0 in the create payload when copying a spool with non-zero usage), 2 new in `InventoryPageCopyButton.test.tsx` (table-row copy button click → "Copy Spool" heading, cards-view copy button click → same heading after switching view modes) — guards against the three call sites drifting apart. Existing `SpoolFormBulk.test.tsx` and `SpoolFormModal.test.tsx` renders that omitted the `mode` prop were updated with the explicit `mode="create"` so the tightened Quick Add gate doesn't hide the toggle from them. Both `InventoryPageCopyButton.test.tsx` and `InventoryPageDeepLink.test.tsx` gained MSW handlers for the modal's open-time fetches (`/api/v1/cloud/status`, `/api/v1/cloud/local-presets`, `/api/v1/cloud/builtin-filaments`, `/api/v1/inventory/color-catalog`, `/api/v1/inventory/spool-catalog`, `/api/v1/printers/`) — without them MSW passes through to the real network, ECONNREFUSEs, and the rejected fetch resolves after the test environment is torn down, surfacing as a flaky "window is not defined" unhandled rejection in the modal's `setLoadingCloudPresets(false)` finally block (pre-existing flake hit ~1 in 3 full-suite runs at PR head). +### Fixed +- **`.bbscfg` Printer Preset Bundle import was broken for every user since launch — sidecar compose file pointed at the wrong branch** ([#1312](https://github.com/maziggy/bambuddy/issues/1312), reported by @hasmar04, confirmed by @netscout2001) — `slicer-api/docker-compose.yml`'s `build.context` pointed at `https://github.com/maziggy/orca-slicer-api.git#bambuddy/profile-resolver`, but the `POST /profiles/bundle` endpoint plus the `uploadBundle` multer middleware were only ever committed to a sibling branch `bambuddy/bundle-import` (commit `a3172c5`, 2026-05-06). Every user who ran the documented `docker compose up -d` got a sidecar without the bundle endpoint — their `POST /profiles/bundle` fell through to the generic `POST /profiles/:category` handler, which either rejected with "Name cannot be empty" (no `name` form field sent) or "Invalid file type. Only JSON files are allowed." (the JSON multer filter rejecting the `.bbscfg`). **Fix:** `bambuddy/bundle-import` fast-forward-merged into `bambuddy/profile-resolver` in the orca-slicer-api repo and pushed, so the compose file's existing branch ref now points at the right commit. No Bambuddy code change. Existing users rebuild with `cd slicer-api/ && docker compose --profile bambu build --no-cache --pull && docker compose --profile bambu up -d` — `--pull` is the key flag because BuildKit caches the git fetch context separately from layer caches, so `--no-cache` alone silently reuses the old branch checkout. New users on 0.2.5+ are unaffected. Lesson on diagnosis flow: the wrong root cause was reported twice during triage before the actual branch mismatch was caught — first as "build a week ago, before the bundle endpoint existed" (correct claim for the wrong branch), then as "rebuild with --pull" (still hit the same bug because the compose file pointed at the branch that never got the work). The reporter's third round of logs — the multer "Only JSON files are allowed" error string from `upload.js:17`, which only matches `uploadJson` not `uploadBundle` — was the smoking gun that no amount of rebuilding would help because the wired-up branch genuinely lacked the endpoint. + +### Changed +- **Support bundle records slicer-API CLI versions; wiki sidecar-update docs hardened** ([#1312](https://github.com/maziggy/bambuddy/issues/1312) follow-up) — Triage scaffolding added during investigation of the bundle-import bug above. Useful independent of that fix: the next time a user reports a sidecar-related failure, the support bundle will identify which slicer CLI version is actually running without needing a manual `curl /health`. **Backend:** new `_fetch_slicer_health(url)` helper in `backend/app/api/routes/support.py` does a 2-second GET on `/health`, parses the JSON, and walks every non-`dataPath` key under `checks` looking for a `version` field — needed because the wrapper labels both bambu-studio-api and orca-slicer-api as `checks.orcaslicer` regardless of which CLI is actually bundled (cosmetic wrapper bug, not Bambuddy's). `_collect_slicer_api_info` now calls it instead of the bare reachability ping and adds two new fields per side to the integrations block: `bambu_studio_version`, `orcaslicer_version`. Captures `"unknown"` verbatim when the wrapper's `--help` regex didn't match (which is itself diagnostic). Behavior preserved on error paths: empty URL returns `None`, connection failure returns `{reachable: False, version: None}`, malformed/non-200 returns `{reachable: True, version: None}` so the reviewer can separate network failure from misconfiguration. Trailing-slash in the configured URL is stripped before appending `/health`. **Tests:** 9 new in `TestFetchSlicerHealth`; existing `TestCollectSlicerApiInfo` tests updated to patch `_fetch_slicer_health` and assert the new `_version` fields. All 62 helper tests pass; ruff clean. **Docs:** `bambuddy-wiki/docs/features/slicer-api.md` got four additions. (1) Quick Start gains a warning callout that the Compose file builds from a branch tip and a plain `docker compose up -d` will keep using the originally-built image. (2) The Updating section now recommends `docker compose --profile bambu build --no-cache --pull` (both flags) and explains why both matter. (3) New troubleshooting entry for the "Name cannot be empty" / "Only JSON files are allowed" `.bbscfg` import error. (4) New troubleshooting entry for the orphan-container conflict (`container name "/bambu-studio-api" is already in use`) that hits users whose existing containers were built from an older compose file with un-prefixed image tags. The pre-existing `/health version: "unknown"` entry also got a note clarifying that the wrapper mislabels the `checks` field as `orcaslicer` for both sidecars — both are cosmetic, not stale-image indicators. + ### Fixed - **LDAP settings: "Advanced" collapsible section header was always rendering in English regardless of UI language** ([#1297](https://github.com/maziggy/bambuddy/issues/1297), reported by @Fuechslein) — `LDAPSettings.tsx:352` calls `t('settings.ldap.advanced') || 'Advanced'`, but the translation key was never defined in any locale file. The `|| 'Advanced'` fallback kicked in and the header rendered as English in every language. Added `settings.ldap.advanced` to all 8 locales: `Advanced` (en), `Erweitert` (de), `Avancé` (fr), `Avanzate` (it), `詳細設定` (ja), `Avançado` (pt-BR), `高级` (zh-CN), `進階` (zh-TW). No component change needed — the fallback now never triggers because the key resolves properly. i18n parity check holds at 4754 leaves across all locales. diff --git a/backend/app/api/routes/support.py b/backend/app/api/routes/support.py index af6f490fe..a45132efb 100644 --- a/backend/app/api/routes/support.py +++ b/backend/app/api/routes/support.py @@ -646,6 +646,45 @@ async def _check_url_reachable(url: str, timeout: float = 2.0) -> bool | None: return False +async def _fetch_slicer_health(url: str, timeout: float = 2.0) -> dict | None: + """Fetch ``/health`` from a slicer sidecar and extract the CLI version. + + Returns ``None`` when ``url`` is empty (so the caller can distinguish + "not configured" from "unreachable"). On any failure to fetch or parse, + returns ``{"reachable": False, "version": None}``. The slicer-API wrapper + labels both sidecars' CLI under ``checks.orcaslicer`` regardless of which + slicer is actually bundled (cosmetic wrapper bug), so we read the version + from whichever non-``dataPath`` child key exists rather than hardcoding + one. This lets the bundle reviewer answer "is the user running the image + they think they are?" without a separate curl round-trip. + """ + if not url or not url.strip(): + return None + health_url = url.rstrip("/") + "/health" + try: + import httpx + + async with httpx.AsyncClient(timeout=timeout, verify=False) as client: # noqa: S501 — local sidecars often use self-signed + r = await client.get(health_url, follow_redirects=False) + if r.status_code != 200: + return {"reachable": True, "version": None} + try: + data = r.json() + except Exception: + return {"reachable": True, "version": None} + checks = data.get("checks") if isinstance(data, dict) else None + if not isinstance(checks, dict): + return {"reachable": True, "version": None} + for key, value in checks.items(): + if key == "dataPath": + continue + if isinstance(value, dict) and "version" in value: + return {"reachable": True, "version": value.get("version")} + return {"reachable": True, "version": None} + except Exception: + return {"reachable": False, "version": None} + + async def _collect_slicer_api_info() -> dict: """Reachability check for configured slicer-API sidecars. @@ -697,12 +736,14 @@ async def _collect_slicer_api_info() -> dict: "orcaslicer_url_source": ("db" if oc_db else ("env_or_default" if oc_url else "unset")), } if info["enabled"]: - bs_reach, oc_reach = await asyncio.gather( - _check_url_reachable(bs_url), - _check_url_reachable(oc_url), + bs_health, oc_health = await asyncio.gather( + _fetch_slicer_health(bs_url), + _fetch_slicer_health(oc_url), ) - info["bambu_studio_reachable"] = bs_reach - info["orcaslicer_reachable"] = oc_reach + info["bambu_studio_reachable"] = (bs_health or {}).get("reachable") if bs_health is not None else None + info["bambu_studio_version"] = (bs_health or {}).get("version") if bs_health is not None else None + info["orcaslicer_reachable"] = (oc_health or {}).get("reachable") if oc_health is not None else None + info["orcaslicer_version"] = (oc_health or {}).get("version") if oc_health is not None else None return info diff --git a/backend/tests/unit/test_support_helpers.py b/backend/tests/unit/test_support_helpers.py index 2e0d25c3e..40a0be978 100644 --- a/backend/tests/unit/test_support_helpers.py +++ b/backend/tests/unit/test_support_helpers.py @@ -715,6 +715,156 @@ class TestCheckUrlReachable: assert result is False +class TestFetchSlicerHealth: + """Tests for the slicer-API health probe that extracts the bundled CLI + version. Knowing the version in the support bundle lets the reviewer + confirm the user is running the image they think they are — exactly the + diagnostic that was missing when issue #1312 surfaced.""" + + def _mock_httpx(self, status_code: int, body): + """Construct a patched httpx.AsyncClient that returns a fixed response.""" + mock_client_cls = MagicMock() + mock_client = AsyncMock() + mock_client_cls.return_value.__aenter__.return_value = mock_client + mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) + mock_response = MagicMock() + mock_response.status_code = status_code + if isinstance(body, Exception): + mock_response.json.side_effect = body + else: + mock_response.json.return_value = body + mock_client.get = AsyncMock(return_value=mock_response) + return mock_client_cls, mock_client + + @pytest.mark.asyncio + async def test_empty_url_returns_none(self): + from backend.app.api.routes.support import _fetch_slicer_health + + assert await _fetch_slicer_health("") is None + assert await _fetch_slicer_health(" ") is None + + @pytest.mark.asyncio + async def test_parses_version_from_orcaslicer_field(self): + """The default sidecar wrapper labels both orca and bambu CLIs under + ``checks.orcaslicer``. The probe must read whichever non-dataPath child + carries a ``version`` field instead of hardcoding the field name.""" + from backend.app.api.routes.support import _fetch_slicer_health + + body = { + "status": "healthy", + "checks": { + "orcaslicer": {"available": True, "version": "2.3.2"}, + "dataPath": {"accessible": True}, + }, + } + mock_client_cls, mock_client = self._mock_httpx(200, body) + with patch("httpx.AsyncClient", mock_client_cls): + result = await _fetch_slicer_health("http://orca:3003") + + assert result == {"reachable": True, "version": "2.3.2"} + # And the URL was actually composed as /health. + mock_client.get.assert_awaited_once() + assert mock_client.get.await_args[0][0] == "http://orca:3003/health" + + @pytest.mark.asyncio + async def test_parses_version_when_wrapper_uses_bambustudio_field(self): + """Future-proofing: if the wrapper is ever fixed to label the bambu CLI + as ``bambustudio``, the probe must still pick up the version. The probe + walks every non-dataPath key looking for a ``version`` field rather + than hardcoding the slicer name.""" + from backend.app.api.routes.support import _fetch_slicer_health + + body = { + "status": "healthy", + "checks": { + "bambustudio": {"available": True, "version": "02.06.00.51"}, + "dataPath": {"accessible": True}, + }, + } + mock_client_cls, _ = self._mock_httpx(200, body) + with patch("httpx.AsyncClient", mock_client_cls): + result = await _fetch_slicer_health("http://bs:3001") + + assert result == {"reachable": True, "version": "02.06.00.51"} + + @pytest.mark.asyncio + async def test_version_unknown_propagates_as_string(self): + """The wrapper emits literal ``"unknown"`` when it can't parse the + slicer's --help output. We surface that as-is — it's diagnostic on + its own (tells the reviewer the regex didn't match).""" + from backend.app.api.routes.support import _fetch_slicer_health + + body = { + "status": "healthy", + "checks": { + "orcaslicer": {"available": True, "version": "unknown"}, + "dataPath": {"accessible": True}, + }, + } + mock_client_cls, _ = self._mock_httpx(200, body) + with patch("httpx.AsyncClient", mock_client_cls): + result = await _fetch_slicer_health("http://bs:3001") + + assert result == {"reachable": True, "version": "unknown"} + + @pytest.mark.asyncio + async def test_non_200_status_is_reachable_but_no_version(self): + """If the URL responds with a non-200, the host is up but the endpoint + isn't the expected one — surface reachable=True so the reviewer can + spot misconfiguration without conflating it with a network failure.""" + from backend.app.api.routes.support import _fetch_slicer_health + + mock_client_cls, _ = self._mock_httpx(404, {}) + with patch("httpx.AsyncClient", mock_client_cls): + result = await _fetch_slicer_health("http://bs:3001") + + assert result == {"reachable": True, "version": None} + + @pytest.mark.asyncio + async def test_malformed_json_returns_reachable_no_version(self): + from backend.app.api.routes.support import _fetch_slicer_health + + mock_client_cls, _ = self._mock_httpx(200, ValueError("not json")) + with patch("httpx.AsyncClient", mock_client_cls): + result = await _fetch_slicer_health("http://bs:3001") + + assert result == {"reachable": True, "version": None} + + @pytest.mark.asyncio + async def test_missing_checks_block_returns_no_version(self): + from backend.app.api.routes.support import _fetch_slicer_health + + mock_client_cls, _ = self._mock_httpx(200, {"status": "healthy"}) + with patch("httpx.AsyncClient", mock_client_cls): + result = await _fetch_slicer_health("http://bs:3001") + + assert result == {"reachable": True, "version": None} + + @pytest.mark.asyncio + async def test_connection_error_returns_unreachable(self): + from backend.app.api.routes.support import _fetch_slicer_health + + with patch("httpx.AsyncClient") as mock_client_cls: + mock_client_cls.return_value.__aenter__.side_effect = ConnectionError("boom") + + result = await _fetch_slicer_health("http://nowhere:9999") + + assert result == {"reachable": False, "version": None} + + @pytest.mark.asyncio + async def test_strips_trailing_slash_before_appending_health(self): + """Defensive: URLs entered with trailing slashes in Settings should + still produce a well-formed /health URL (no double-slash).""" + from backend.app.api.routes.support import _fetch_slicer_health + + body = {"status": "healthy", "checks": {"orcaslicer": {"available": True, "version": "2.3.2"}}} + mock_client_cls, mock_client = self._mock_httpx(200, body) + with patch("httpx.AsyncClient", mock_client_cls): + await _fetch_slicer_health("http://bs:3001/") + + assert mock_client.get.await_args[0][0] == "http://bs:3001/health" + + class TestCollectSlicerApiInfo: """Tests for the slicer-API info block (configured URLs + reachability). @@ -744,24 +894,28 @@ class TestCollectSlicerApiInfo: session_ctx = self._make_settings_session({"use_slicer_api": "false", "preferred_slicer": "bambu_studio"}) with ( patch("backend.app.api.routes.support.async_session", session_ctx), - patch("backend.app.api.routes.support._check_url_reachable") as mock_check, + patch("backend.app.api.routes.support._fetch_slicer_health") as mock_health, ): info = await _collect_slicer_api_info() - mock_check.assert_not_called() + mock_health.assert_not_called() assert info["enabled"] is False assert info["preferred"] == "bambu_studio" assert info["bambu_studio_url_set_in_db"] is False assert info["orcaslicer_url_set_in_db"] is False assert "bambu_studio_reachable" not in info assert "orcaslicer_reachable" not in info + assert "bambu_studio_version" not in info + assert "orcaslicer_version" not in info @pytest.mark.asyncio async def test_enabled_runs_reachability_check_for_both_urls(self): from backend.app.api.routes.support import _collect_slicer_api_info - async def fake_check(url, timeout=2.0): - return "orca" in url + async def fake_health(url, timeout=2.0): + if "orca" in url: + return {"reachable": True, "version": "2.3.2"} + return {"reachable": False, "version": None} session_ctx = self._make_settings_session( { @@ -773,7 +927,7 @@ class TestCollectSlicerApiInfo: ) with ( patch("backend.app.api.routes.support.async_session", session_ctx), - patch("backend.app.api.routes.support._check_url_reachable", side_effect=fake_check), + patch("backend.app.api.routes.support._fetch_slicer_health", side_effect=fake_health), ): info = await _collect_slicer_api_info() @@ -784,6 +938,8 @@ class TestCollectSlicerApiInfo: assert info["orcaslicer_url_source"] == "db" assert info["bambu_studio_reachable"] is False assert info["orcaslicer_reachable"] is True + assert info["bambu_studio_version"] is None + assert info["orcaslicer_version"] == "2.3.2" @pytest.mark.asyncio async def test_env_var_fallback_url_pinged_when_db_setting_empty(self): @@ -799,16 +955,16 @@ class TestCollectSlicerApiInfo: seen_urls: list[str] = [] - async def fake_check(url, timeout=2.0): + async def fake_health(url, timeout=2.0): seen_urls.append(url) - return True + return {"reachable": True, "version": "02.06.00.51"} # DB has use_slicer_api=true but NO bambu_studio_api_url row, simulating # a user who set the URL via the BAMBU_STUDIO_API_URL env var. session_ctx = self._make_settings_session({"use_slicer_api": "true", "preferred_slicer": "bambu_studio"}) with ( patch("backend.app.api.routes.support.async_session", session_ctx), - patch("backend.app.api.routes.support._check_url_reachable", side_effect=fake_check), + patch("backend.app.api.routes.support._fetch_slicer_health", side_effect=fake_health), patch("backend.app.api.routes.support.settings") as mock_app_settings, ): # Pydantic-settings would normally do this for us when reading the @@ -824,6 +980,7 @@ class TestCollectSlicerApiInfo: assert info["bambu_studio_url_set_in_db"] is False assert info["bambu_studio_url_source"] == "env_or_default" assert info["bambu_studio_reachable"] is True + assert info["bambu_studio_version"] == "02.06.00.51" @pytest.mark.asyncio async def test_reachability_uses_unredacted_url(self): @@ -835,9 +992,9 @@ class TestCollectSlicerApiInfo: seen_urls: list[str] = [] - async def fake_check(url, timeout=2.0): + async def fake_health(url, timeout=2.0): seen_urls.append(url) - return True + return {"reachable": True, "version": "unknown"} session_ctx = self._make_settings_session( { @@ -848,7 +1005,7 @@ class TestCollectSlicerApiInfo: ) with ( patch("backend.app.api.routes.support.async_session", session_ctx), - patch("backend.app.api.routes.support._check_url_reachable", side_effect=fake_check), + patch("backend.app.api.routes.support._fetch_slicer_health", side_effect=fake_health), ): await _collect_slicer_api_info()