Register a Spoolman extra field with its own write (issue #2903)

Spoolman rejects a spool whose extra dict carries a key it has not been
    told about, answering 400 "Unknown extra field tag.". Bambuddy keeps the
    tray UUID in extra.tag, so that key has to exist before the first spool
    is created. Registration ran from three hand-maintained lists that fire
    when the integration is set up -- the connect route, startup, and two
    inline blocks in the inventory routes. Enabling Spoolman from the
    Settings page reaches none of them, so the first AMS sync on a fresh
    Spoolman failed on every slot while vendor and filament creation
    succeeded.

    Neither fix suggested on the issue is quite the right shape. Adding the
    block to PUT /settings/spoolman fixes this path and makes a third copy
    of a list that has already drifted -- it would still omit
    bambu_color_name. Ensuring at the sync entry point leaves the other four
    tag writers alone: linking and unlinking a tag, and both inventory edit
    paths.

    So the registration moved to the write. create_spool, update_spool and
    update_spool_full each register the keys of the extra dict they are
    about to send, once per client, before sending. Every tag writer funnels
    through one of the three, merge_spool_extra included. This closes the
    class rather than the instance: a write that carries a key is a write
    that registers it, and bambu_color_name shows why that matters -- it
    never made it into the connect or startup lists at all, and works today
    only because two call sites remembered it by hand.

    Best-effort, deliberately. ensure_extra_field already logs and returns
    False rather than raising, so a registration that fails leaves the write
    to be attempted and to report exactly what it reported before. Failures
    are not memoised either, so a Spoolman that was merely restarting gets
    another try on the next write.

    The older blocks stay. They are redundant now, but the inventory routes'
    inline calls are pinned by tests that assert them against a mocked
    client, where the funnel cannot run.

    The status endpoint is the other half. The Connect button would have
    registered the fields, and the reason nobody reaches it is that
    GET /spoolman/status reported "connected" whenever an earlier request
    had left a client object behind. Roughly twenty call sites build one
    lazily, and saving the Settings page builds one as a side effect of
    syncing locations, so the flag turned on which page had been opened
    rather than on anything about Spoolman. The UI reads it twice -- Connect
    only while disconnected, the sync section only while connected -- so
    those two controls landed in states the user cannot explain. It now asks
    the Spoolman that is configured, including the stale-URL check every
    other route already does, and does not probe at all when the integration
    is switched off, which used to let a leftover client report a disabled
    Spoolman as connected.

    That leaves nothing for Disconnect to do, so it is gone. Spoolman is a
    stateless HTTP API with no session to close; the button dropped the
    client object, the next request rebuilt it lazily, and the status
    flipped back on its own within the 30s poll -- an action that looked
    like it worked and then quietly undid itself. The enable toggle owns
    turning the integration off. Connect stays as what it always was in
    practice, a way to re-check a Spoolman that is not answering, and is
    shown only then. Its translation keys are left in place; only the
    control is removed.

    Resolving the client there means the status poll can now fail in ways a
    read-only check could not, so it no longer reports failure by failing.
    Replacing a client closes the previous one and httpx's aclose() is not
    guaranteed not to raise; a poll that runs every 30 seconds answering 500
    is worse than one answering what is true either way, which is that
    Spoolman could not be reached. The SSRF rejection keeps its own message
    rather than folding into the general one -- a URL the guard refuses is
    the admin's to correct, and that is only actionable if the log says so.

    Tests drive the real client against a fake Spoolman that enforces the
    unknown-extra-field rule rather than mocking it away, so each one fails
    against the old code for the reason the reporter's install did. The
    concurrency test needed the fake to be async: MockTransport answers
    without suspending, so the first version ran each request to completion
    in turn and passed just as happily with the lock removed.

CHANGELOG.md

@@ -15,6 +15,8 @@ All notable changes to Bambuddy will be documented in this file.
 - **An archive that arrives with only a name now says what to do about it (#2843, reporter @gyrene2083)** — The no-3MF banner and the connection diagnostic both explained why a print archived with nothing but a name and offered nothing to do about it, and the explanation was wrong in the way that mattered most: both blamed the printer's firmware and said no setting changes it, which reads as "your machine is broken and nothing will help". Measured on hardware: same Bambu Studio, same model, three printers a minute apart, all reporting the external-storage option as on — the H2C and H2D went to internal storage and archived with a name only, the X1C went to the card and archived in full. The same H2C and H2D sliced in OrcaSlicer put the file on the card and archived in full, and turning the option off changed nothing, because OrcaSlicer always uploads over FTPS. So the destination is the slicer's choice, not the printer's, and the option governs neither slicer on this generation. Both strings now name Bambu Studio rather than the firmware and lead with the two routes that take one step — start the print from Bambuddy, or slice in OrcaSlicer — with Send-with-External followed by a separate print start offered as the way to stay in Bambu Studio. Both also mention that a card or stick is still needed, since with an empty slot OrcaSlicer refuses to send at all. Translated in all thirteen locales, and the README callout and this entry now link the upstream issue, bambulab/BambuStudio#10481, so the behaviour can be followed where it is tracked.
 
 ### Fixed
+- **The first AMS sync after enabling Spoolman from Settings failed on every slot (#2903, diagnosed by @ojimpo)** — Spoolman rejects a spool whose `extra` dict carries a key it has not been told about, answering HTTP 400 `Unknown extra field tag.`. Bambuddy stores the tray UUID in `extra.tag`, so that key has to be registered before the first spool is created — and registration only ever ran from three hand-maintained lists that fire when the integration is *set up*: the Connect button, application startup, and two inline blocks in the inventory routes. Enabling Spoolman from the Settings page reaches none of them, so "Sync AMS Data" reported `Synced 0 spools with 3 errors` with vendor and filament creation succeeding and only spool creation rejected. Restarting Bambuddy cleared it, which made the failure look like a connectivity problem. The Connect button would also have fixed it, but it is not on screen by then: saving the settings initialises the Spoolman client as a side effect of syncing locations, the status endpoint reads any live client as "connected", and the UI shows the Connect button only while disconnected — so the one registration path reachable from the interface hides itself exactly when it is needed. Registration now travels with the write instead of the feature: every spool write that carries an `extra` dict registers the keys it is about to send, once per client, before it sends them. That covers all five places Bambuddy writes a tag — AMS sync, linking and unlinking a tag from a spool, and both inventory edit paths — and it closes the class rather than the instance, since a write that carries a key is now a write that registers it. `bambu_color_name` is the cautionary case: it never made it into the Connect or startup lists at all, and worked only because two call sites remembered to register it by hand. Registration stays best-effort — if it fails, the write is still attempted and reports exactly what it reported before, and the failure is not cached, so a Spoolman that was merely restarting is retried on the next write.
+- **The Spoolman connection status described Bambuddy's memory rather than Spoolman (#2903)** — "Connected" meant "some earlier request in this process left a client object behind", and around twenty code paths build one lazily, so the answer turned on which page had been opened rather than on anything about Spoolman. A switched-off integration could still report Connected from a leftover client; changing the URL kept reporting on the previous host; and saving the Settings page built a client as a side effect of syncing locations, which is how enabling Spoolman came to report Connected before anything had been set up. The status now asks the Spoolman that is configured right now, so the answer is the same whatever you opened first. The Disconnect button is gone with it: Spoolman is a stateless HTTP API with no session to close, so the button only dropped the client object that the next request rebuilt moments later, after which the status flipped back on its own — it looked like it had worked, then quietly undid itself. Turning the integration off is the enable toggle's job, and Connect remains as what it actually is, a way to re-check a Spoolman that is not answering.
 - **An AMS slot assigned a PLA+ spool became unusable for PLA (#2902, reported by @doncaruana)** — Assigning a spool wrote its material straight into the slot's `tray_type`, and a slot that says "PLA+" satisfies nothing that asks for PLA: not OrcaSlicer, not Bambu Studio, and not Bambuddy's own dispatch matcher, which compares the printer's reported type to the one the 3MF declares. The same string then missed the filament-id lookup, so the slot went out with no `tray_info_idx` at all — the half-configured state a printer reverts from — and took the generic 200/240°C nozzle range instead of PLA's. PLA+ is not a special case: Bambuddy's own colour catalogue supplies the material dropdown, and about forty of its values are vendor product lines rather than filament types — HTPLA, PolyTerra PLA, PLA Matte, ASA Extrafill, Flexfill TPU 98A. All four routes that configure a slot now reduce the material to a type the printer knows before sending it, and the product name moves to `tray_sub_brands`, which is where Bambu Lab itself puts it — their catalogue has a preset named "eSUN PLA+" whose type is PLA. A material that cannot be placed is sent exactly as before rather than guessed at, so this can only repair a slot, never break a working one; and a material that already had a preset of its own keeps it, so "PETG HF" is not quietly downgraded to plain PETG. One thing that starts working as a result: a slot holding a calibrated preset is now reused when a same-material spool is assigned to it, which for these spools could never happen before.
 - **A failed upload told you to check the SD card, whatever had actually gone wrong (#2899, reported by @grolmus)** — Every dispatch upload that failed carried the same sentence: "Failed to upload file to printer. Check if SD card is inserted and properly formatted (FAT32/exFAT)." The reporter got it after a TLS handshake failure and restarted the printer on the strength of it. That could not have helped — the handshake never reached the printer's filesystem, and the cool-off that made the next dispatch fail identically lives in Bambuddy's own memory, where power-cycling a printer does not reach. #2780 had already taken operator advice out of this failure's log line, for exactly the reason that the advice was known not to work; it survived in the string people actually read. The information to say something true was never missing. The FTP client separates five connect failures and three upload reply codes, each with its own log line — 553 even gets a spelled-out list of storage causes — and then handed the caller a bare true-or-false, so the dispatch had nothing to go on and guessed storage for all of them. The reason now travels with the result, and the message is chosen from it: a 553 or 552 keeps the card advice, which is the case it was written for, and quotes the printer's reply code so a queue entry and a support bundle can be lined up. A handshake failure says the file service answered without TLS and that the card is not involved. A refused connection points at the access code, a timeout at the network, and anything the client could not classify says so and points at the log rather than picking a plausible cause — a wrong instruction costs more than a vague one, because it sends someone to work on hardware that is fine. No message prescribes a power cycle, which is the restraint #2780 settled on. The failure notification carries the same sentence the queue shows, rather than its own fixed "Failed to upload file to printer", so a push and the screen can no longer disagree.
 - **One handshake failure took out three queued jobs and every retry they had (#2898, reported by @grolmus)** — A print dispatch that met a TLS handshake failure spent its whole retry budget without opening a single socket. The cool-off that a failed handshake arms (#2780) lives inside `connect()`, so it applied to everything — including the dispatch, whose four attempts two seconds apart were all answered from the gate rather than the network. The reporter's farm logs the shape exactly: the delete that clears the way for the upload took the SSL error at 11:10:04.956, the upload's first attempt started 8ms later, and attempts two through four took one to two milliseconds each. Because the cool-off runs for five minutes, the next two jobs queued for that printer failed the same way inside the same window, and on that farm the failure is transient — a manual connect a second later completes cleanly — so the retry the gate suppressed is the one that would have worked. The gate was serving two callers that want opposite things from it. The background sweeps that fetch a 3MF, a cover or a timelapse after a print walk about a hundred and ten candidate paths against one wedged printer with nobody waiting, and backing off for minutes is right for them; they keep today's behaviour untouched. A dispatch is one delete plus at most four upload attempts with someone watching a progress bar, so it now ignores the cool-off, as does a firmware upload, for the same reason. Callers that do respect the cool-off no longer sleep out a retry loop against it either: the loop stops at the attempt that armed the gate and says so, instead of spending three more attempts and six seconds on connections that cannot happen. What made this a log dive rather than a glance is fixed with it: the cool-off skip was the one connect failure that reported without naming its cause, and did so at debug level, so four identical reason-free warnings were all the operator saw. It now says at warning level that nothing was sent and how long the printer has left — once per cool-off rather than once per attempt, so that raising it does not recreate the log flood #2780 set out to stop.

backend/app/api/routes/spoolman.py

@@ -101,14 +101,47 @@ async def get_spoolman_status(
     db: AsyncSession = Depends(get_db),
     _: User | None = RequirePermissionIfAuthEnabled(Permission.FILAMENTS_READ),
 ):
-    """Get Spoolman integration status."""
+    """Get Spoolman integration status.
+
+    ``connected`` answers "does the configured Spoolman respond?", which means
+    asking it. It used to answer "has some earlier request in this process left
+    a client object lying around?" -- and roughly twenty call sites build one
+    lazily, so the answer depended on which page happened to load first rather
+    than on anything about Spoolman.
+
+    That mattered because the UI reads this one flag twice: it offers Connect
+    only while disconnected, and the AMS sync section only while connected.
+    Saving the Settings page initialises a client as a side effect of syncing
+    locations, so enabling Spoolman there reported "connected" without anything
+    having been set up, hiding the Connect button and revealing a sync that then
+    failed on every slot (issue #2903). Registration no longer depends on that
+    button, but the flag was still describing Bambuddy's memory rather than the
+    integration, so it is now resolved the same way every other route resolves
+    it -- including the stale-URL check, so editing the URL is not reported
+    against the old host.
+    """
     sm = await get_spoolman_settings(db)
     enabled, url = sm["enabled"], sm["url"]
 
-    client = await get_spoolman_client()
     connected = False
-    if client:
-        connected = await client.health_check()
+    if enabled and url:
+        client = await get_spoolman_client()
+        if not client or client.base_url != url.rstrip("/"):
+            try:
+                client = await init_spoolman_client(url)
+            except ValueError as exc:
+                logger.warning("Spoolman URL %r rejected by SSRF guard during status check: %s", url, exc)
+                client = None
+            except Exception as exc:
+                # Every remaining way this can fail still answers the question:
+                # replacing a client closes the previous one, and httpx's
+                # aclose() is not guaranteed not to raise. A status poll that
+                # 500s every 30 seconds is worse than one reporting what is
+                # true either way -- that Spoolman could not be reached.
+                logger.warning("Could not open a Spoolman client for %r during status check: %s", url, exc)
+                client = None
+        if client:
+            connected = await client.health_check()
 
     return SpoolmanStatus(
         enabled=enabled,

backend/app/services/spoolman.py

@@ -106,6 +106,12 @@ class SpoolmanClient:
         # Per-spool locks for atomic read-modify-write in merge_spool_extra.
         # WeakValueDictionary: locks are GC'd once no coroutine holds a reference.
         self._extra_locks: weakref.WeakValueDictionary[int, asyncio.Lock] = weakref.WeakValueDictionary()
+        # Extra-field names this client has already registered with Spoolman.
+        # Bounded by the number of distinct keys Bambuddy writes, so it never
+        # grows with spool count; scoped to the instance so a client pointed at
+        # a different Spoolman starts over.
+        self._ensured_extra_fields: set[str] = set()
+        self._ensure_extra_lock = asyncio.Lock()
 
     async def _get_client(self) -> httpx.AsyncClient:
         """Get or create the HTTP client with connection pooling limits."""
@@ -403,6 +409,7 @@ class SpoolmanClient:
             data["comment"] = comment
         if extra:
             data["extra"] = extra
+            await self._ensure_extra_fields(extra)
 
         logger.debug("Creating spool in Spoolman: %s", data)
         try:
@@ -443,6 +450,7 @@ class SpoolmanClient:
             data["location"] = location
         if extra:
             data["extra"] = extra
+            await self._ensure_extra_fields(extra)
         data["last_used"] = datetime.now(timezone.utc).isoformat()
 
         response = await self._request_spool("PATCH", spool_id, json_body=data, operation="update")
@@ -700,6 +708,7 @@ class SpoolmanClient:
             data["location"] = location
         if extra is not None:
             data["extra"] = extra
+            await self._ensure_extra_fields(extra)
         if clear_spool_weight:
             data["spool_weight"] = None
         elif spool_weight is not None:
@@ -951,6 +960,7 @@ class SpoolmanClient:
             response = await client.get(f"{self.api_url}/field/spool/{name}")
             if response.status_code == 200:
                 logger.debug("Spoolman extra field %r already exists", name)
+                self._ensured_extra_fields.add(name)
                 return True
 
             # Field doesn't exist - create it
@@ -962,6 +972,7 @@ class SpoolmanClient:
             response = await client.post(f"{self.api_url}/field/spool/{name}", json=field_data)
             if response.status_code in (200, 201):
                 logger.info("Created Spoolman extra field %r", name)
+                self._ensured_extra_fields.add(name)
                 return True
 
             logger.warning(
@@ -976,6 +987,43 @@ class SpoolmanClient:
             logger.warning("Failed to ensure Spoolman extra field %r exists: %s", name, e)
             return False
 
+    async def _ensure_extra_fields(self, extra: dict | None) -> None:
+        """Register every extra key an outgoing write declares, once per client.
+
+        Spoolman answers HTTP 400 "Unknown extra field <name>." for any extra
+        key that was not registered first, so registration has to happen before
+        the write, not before the feature. It used to happen before the feature:
+        three hand-maintained lists (the connect route, startup, and two inline
+        blocks in the inventory routes) each named the fields they expected to
+        be written later. Enabling Spoolman from Settings reaches none of them,
+        so the first AMS sync on a fresh Spoolman failed on every slot -- and
+        the Connect button that would have registered them is hidden by then,
+        because saving the settings initialises the client and the status
+        endpoint reads that as "connected" (issue #2903).
+
+        Keying off the payload instead removes the chance to forget: a write
+        that carries a key is a write that registers it. ``bambu_color_name``
+        is the cautionary case -- it never made it into the connect or startup
+        lists, and only works today because two call sites remembered to
+        register it by hand.
+
+        Best-effort by design. ``ensure_extra_field`` logs and returns False
+        rather than raising, and a failure here must not turn a write that
+        might still succeed into one that never happens -- the caller's own
+        error handling stays exactly as it was.
+        """
+        names = [name for name in (extra or {}) if name not in self._ensured_extra_fields]
+        if not names:
+            return
+
+        async with self._ensure_extra_lock:
+            for name in names:
+                # Re-check under the lock: a concurrent write may have just
+                # registered this one, and two syncs racing to POST the same
+                # field is how one of them gets a needless warning logged.
+                if name not in self._ensured_extra_fields:
+                    await self.ensure_extra_field(name)
+
     def parse_ams_tray(self, ams_id: int, tray_data: dict) -> AMSTray | None:
         """Parse raw MQTT tray data into an AMSTray; returns None for empty or invalid trays."""
         # Skip empty trays - check for valid tray_type

backend/tests/integration/test_spoolman_status_2903.py

@@ -0,0 +1,201 @@
+"""``connected`` describes Spoolman, not this process's memory (issue #2903).
+
+``GET /spoolman/status`` used to report ``connected`` by looking for a client
+object left behind by some earlier request. Around twenty call sites build one
+lazily, so the answer turned on which page had been loaded rather than on
+anything about Spoolman -- and the Settings page builds one as a side effect of
+saving, which is how enabling the integration came to report "connected" before
+anything had been set up.
+
+The UI reads the flag twice, offering the Connect button only while
+disconnected and the AMS sync section only while connected, so an answer that
+depends on request ordering puts those two controls into states the user cannot
+predict or explain.
+"""
+
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+from httpx import AsyncClient
+
+
+@pytest.fixture
+async def spoolman_enabled(db_session):
+    from backend.app.models.settings import Settings
+
+    db_session.add(Settings(key="spoolman_enabled", value="true"))
+    db_session.add(Settings(key="spoolman_url", value="http://localhost:7912"))
+    await db_session.commit()
+
+
+@pytest.fixture
+async def spoolman_disabled_but_configured(db_session):
+    from backend.app.models.settings import Settings
+
+    db_session.add(Settings(key="spoolman_enabled", value="false"))
+    db_session.add(Settings(key="spoolman_url", value="http://localhost:7912"))
+    await db_session.commit()
+
+
+def _client(*, healthy: bool = True, base_url: str = "http://localhost:7912") -> MagicMock:
+    client = MagicMock()
+    client.base_url = base_url
+    client.health_check = AsyncMock(return_value=healthy)
+    return client
+
+
+def _patch(get_returns, init_returns=None, init_side_effect=None):
+    """Patch the route module's client accessors."""
+    init = AsyncMock(return_value=init_returns, side_effect=init_side_effect)
+    return (
+        patch("backend.app.api.routes.spoolman.get_spoolman_client", AsyncMock(return_value=get_returns)),
+        patch("backend.app.api.routes.spoolman.init_spoolman_client", init),
+        init,
+    )
+
+
+class TestItAsksSpoolmanRatherThanItself:
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_it_reports_connected_without_a_prior_client(self, async_client: AsyncClient, spoolman_enabled):
+        """Nothing has built a client yet -- the status must still be the truth."""
+        healthy = _client()
+        get_patch, init_patch, init = _patch(None, init_returns=healthy)
+
+        with get_patch, init_patch:
+            response = await async_client.get("/api/v1/spoolman/status")
+
+        assert response.status_code == 200
+        assert response.json()["connected"] is True
+        init.assert_awaited_once_with("http://localhost:7912")
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_it_asks_the_url_configured_now_not_the_one_cached(self, async_client: AsyncClient, spoolman_enabled):
+        """A client left pointing at the previous URL must not answer for the new one."""
+        stale = _client(base_url="http://old-host:7912")
+        fresh = _client()
+        get_patch, init_patch, init = _patch(stale, init_returns=fresh)
+
+        with get_patch, init_patch:
+            response = await async_client.get("/api/v1/spoolman/status")
+
+        assert response.json()["connected"] is True
+        init.assert_awaited_once_with("http://localhost:7912")
+        stale.health_check.assert_not_awaited()
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_a_matching_client_is_reused(self, async_client: AsyncClient, spoolman_enabled):
+        existing = _client()
+        get_patch, init_patch, init = _patch(existing)
+
+        with get_patch, init_patch:
+            response = await async_client.get("/api/v1/spoolman/status")
+
+        assert response.json()["connected"] is True
+        init.assert_not_awaited()
+        existing.health_check.assert_awaited_once()
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_an_unreachable_spoolman_reports_disconnected(self, async_client: AsyncClient, spoolman_enabled):
+        """The Connect button is a retry affordance, so this is the case that shows it."""
+        get_patch, init_patch, _ = _patch(_client(healthy=False))
+
+        with get_patch, init_patch:
+            response = await async_client.get("/api/v1/spoolman/status")
+
+        assert response.json() == {
+            "enabled": True,
+            "connected": False,
+            "url": "http://localhost:7912",
+        }
+
+
+class TestItStaysQuietWhenThereIsNothingToAsk:
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_a_disabled_integration_is_never_probed(
+        self, async_client: AsyncClient, spoolman_disabled_but_configured
+    ):
+        """A stale client used to make a switched-off integration report "Connected"."""
+        leftover = _client()
+        get_patch, init_patch, init = _patch(leftover)
+
+        with get_patch, init_patch:
+            response = await async_client.get("/api/v1/spoolman/status")
+
+        assert response.json()["enabled"] is False
+        assert response.json()["connected"] is False
+        leftover.health_check.assert_not_awaited()
+        init.assert_not_awaited()
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_no_url_configured_is_not_probed(self, async_client: AsyncClient, db_session):
+        from backend.app.models.settings import Settings
+
+        db_session.add(Settings(key="spoolman_enabled", value="true"))
+        await db_session.commit()
+        get_patch, init_patch, init = _patch(None)
+
+        with get_patch, init_patch:
+            response = await async_client.get("/api/v1/spoolman/status")
+
+        assert response.json()["connected"] is False
+        init.assert_not_awaited()
+
+
+class TestWhenTheUrlCannotBeUsed:
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_an_ssrf_rejected_url_reports_disconnected_rather_than_erroring(
+        self, async_client: AsyncClient, spoolman_enabled
+    ):
+        """The guard raises ValueError; a status poll must not become a 500."""
+        get_patch, init_patch, _ = _patch(None, init_side_effect=ValueError("blocked"))
+
+        with get_patch, init_patch:
+            response = await async_client.get("/api/v1/spoolman/status")
+
+        assert response.status_code == 200
+        assert response.json()["connected"] is False
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_the_ssrf_rejection_says_so_rather_than_reading_as_a_generic_fault(
+        self, async_client: AsyncClient, spoolman_enabled, caplog
+    ):
+        """A rejected URL is the admin's to fix, so the log has to name it.
+
+        Both failure branches return the same body, so behaviour alone cannot
+        tell them apart -- only the line each one logs can, and a URL the guard
+        refuses needs different words from a client that would not open.
+        """
+        get_patch, init_patch, _ = _patch(None, init_side_effect=ValueError("blocked"))
+
+        with caplog.at_level("WARNING"), get_patch, init_patch:
+            await async_client.get("/api/v1/spoolman/status")
+
+        assert "SSRF guard" in caplog.text
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_a_client_that_cannot_be_opened_reports_disconnected(
+        self, async_client: AsyncClient, spoolman_enabled, caplog
+    ):
+        """Replacing a client closes the old one, and httpx's aclose() may raise.
+
+        A poll that runs every 30 seconds must not answer 500 when it can
+        answer the truth instead -- and must still say why in the log.
+        """
+        get_patch, init_patch, _ = _patch(None, init_side_effect=RuntimeError("event loop is closed"))
+
+        with caplog.at_level("WARNING"), get_patch, init_patch:
+            response = await async_client.get("/api/v1/spoolman/status")
+
+        assert response.status_code == 200
+        assert response.json()["connected"] is False
+        assert "Could not open a Spoolman client" in caplog.text
+        assert "SSRF guard" not in caplog.text

backend/tests/unit/services/test_spoolman_extra_field_registration_2903.py

@@ -0,0 +1,315 @@
+"""Extra-field registration travels with the write that needs it (issue #2903).
+
+Spoolman rejects any spool payload carrying an ``extra`` key it has not been
+told about, answering HTTP 400 ``Unknown extra field <name>.``. Bambuddy used
+to register those keys from three hand-maintained lists that ran when the
+integration was *set up* -- the connect route, application startup, and two
+inline blocks in the inventory routes. Enabling Spoolman from Settings runs
+none of them, so the first "Sync AMS Data" against a fresh Spoolman failed on
+every slot.
+
+The fake below is the point of these tests: it enforces Spoolman's rule rather
+than mocking it away, so every test here fails against the old code for the
+same reason the reporter's install did.
+"""
+
+import asyncio
+import json
+
+import httpx
+import pytest
+
+from backend.app.services.spoolman import AMSTray, SpoolmanClient
+
+
+class FakeSpoolman:
+    """A Spoolman that rejects unregistered extra keys, the way the real one does."""
+
+    def __init__(self, *, registered: set[str] | None = None, field_status: int = 200):
+        self.registered: set[str] = set(registered or ())
+        # Lets a test make registration fail without breaking anything else.
+        self.field_status = field_status
+        self.spools: dict[int, dict] = {}
+        self.log: list[str] = []
+        self._next_id = 1
+
+    def _reject_unknown_extra(self, body: dict) -> httpx.Response | None:
+        for name in body.get("extra") or {}:
+            if name not in self.registered:
+                return httpx.Response(400, json={"message": f"Unknown extra field {name}."})
+        return None
+
+    async def handler(self, request: httpx.Request) -> httpx.Response:
+        # Yield to the event loop on every call, so two coroutines driving this
+        # fake genuinely interleave. Without it MockTransport answers without
+        # ever suspending, and a "concurrent" test runs each request to
+        # completion in turn -- proving nothing about the locking below.
+        await asyncio.sleep(0)
+        path = request.url.path.removeprefix("/api/v1")
+        self.log.append(f"{request.method} {path}")
+        body = json.loads(request.content) if request.content else {}
+
+        if path.startswith("/field/spool/"):
+            name = path.rsplit("/", 1)[-1]
+            if request.method == "GET":
+                if self.field_status != 200:
+                    return httpx.Response(self.field_status)
+                return httpx.Response(200) if name in self.registered else httpx.Response(404)
+            if self.field_status != 200:
+                return httpx.Response(self.field_status)
+            self.registered.add(name)
+            return httpx.Response(200, json={"name": name})
+
+        if path == "/spool" and request.method == "POST":
+            if (rejection := self._reject_unknown_extra(body)) is not None:
+                return rejection
+            spool = {"id": self._next_id, **body}
+            self.spools[self._next_id] = spool
+            self._next_id += 1
+            return httpx.Response(200, json=spool)
+
+        if path.startswith("/spool/"):
+            spool_id = int(path.rsplit("/", 1)[-1])
+            if request.method == "GET":
+                return httpx.Response(200, json=self.spools[spool_id])
+            if (rejection := self._reject_unknown_extra(body)) is not None:
+                return rejection
+            self.spools[spool_id].update(body)
+            return httpx.Response(200, json=self.spools[spool_id])
+
+        if path == "/vendor":
+            if request.method == "GET":
+                return httpx.Response(200, json=[{"id": 1, "name": "Bambu Lab"}])
+            return httpx.Response(200, json={"id": 1, "name": body.get("name", "")})
+
+        if path == "/filament":
+            if request.method == "GET":
+                return httpx.Response(200, json=[])
+            return httpx.Response(200, json={"id": 7, **body})
+
+        if path == "/external/filament":
+            return httpx.Response(200, json=[])
+
+        return httpx.Response(200, json=[])
+
+    def field_calls(self, name: str) -> list[str]:
+        return [entry for entry in self.log if entry.endswith(f"/field/spool/{name}")]
+
+
+def _client(fake: FakeSpoolman) -> SpoolmanClient:
+    client = SpoolmanClient("https://spoolman.test")
+    client._client = httpx.AsyncClient(transport=httpx.MockTransport(fake.handler))
+    return client
+
+
+def _tray(tray_uuid: str) -> AMSTray:
+    return AMSTray(
+        ams_id=0,
+        tray_id=0,
+        tray_type="PLA",
+        tray_sub_brands="PLA Basic",
+        tray_color="000000FF",
+        remain=100,
+        tag_uid="",
+        tray_uuid=tray_uuid,
+        tray_info_idx="GFA00",
+        tray_weight=1000,
+    )
+
+
+class TestTheReportedCase:
+    """A fresh Spoolman, a fresh Bambuddy, and the first AMS sync."""
+
+    @pytest.mark.asyncio
+    async def test_syncing_a_slot_no_longer_fails_on_a_fresh_spoolman(self):
+        fake = FakeSpoolman()  # GET /field/spool returns nothing: no custom fields at all
+        client = _client(fake)
+
+        result = await client.sync_ams_tray(_tray("D144798DEF394926ACAE9D69ABA910CC"), "OJIMPO-X2D-01")
+
+        assert result is not None, "spool creation was rejected -- this is the reported 400"
+        assert result["extra"]["tag"] == json.dumps("D144798DEF394926ACAE9D69ABA910CC")
+        assert "tag" in fake.registered
+
+    @pytest.mark.asyncio
+    async def test_all_three_slots_sync_rather_than_erroring(self):
+        """The report's exact shape: "Synced 0 spools with 3 errors"."""
+        fake = FakeSpoolman()
+        client = _client(fake)
+        tags = [
+            "D144798DEF394926ACAE9D69ABA910CC",
+            "1880BE1371014F4CA951BE6A30C99E44",
+            "1D1F3C49046246DBBADBC3631B7F1B61",
+        ]
+
+        synced = [await client.sync_ams_tray(_tray(tag), "OJIMPO-X2D-01") for tag in tags]
+
+        assert all(s is not None for s in synced)
+        assert [s["extra"]["tag"] for s in synced] == [json.dumps(t) for t in tags]
+
+    @pytest.mark.asyncio
+    async def test_the_field_is_registered_before_the_spool_is_posted(self):
+        """Ordering is the whole fix -- registering afterwards rescues nothing."""
+        fake = FakeSpoolman()
+        client = _client(fake)
+
+        await client.create_spool(filament_id=7, extra={"tag": json.dumps("ABC")})
+
+        assert fake.log.index("POST /field/spool/tag") < fake.log.index("POST /spool")
+
+
+class TestItAsksSpoolmanOnlyOnce:
+    @pytest.mark.asyncio
+    async def test_a_second_write_does_not_ask_again(self):
+        fake = FakeSpoolman()
+        client = _client(fake)
+
+        await client.create_spool(filament_id=7, extra={"tag": json.dumps("A")})
+        await client.create_spool(filament_id=7, extra={"tag": json.dumps("B")})
+
+        assert fake.field_calls("tag") == ["GET /field/spool/tag", "POST /field/spool/tag"]
+
+    @pytest.mark.asyncio
+    async def test_an_already_registered_field_is_never_created(self):
+        fake = FakeSpoolman(registered={"tag"})
+        client = _client(fake)
+
+        await client.create_spool(filament_id=7, extra={"tag": json.dumps("A")})
+
+        assert fake.field_calls("tag") == ["GET /field/spool/tag"]
+
+    @pytest.mark.asyncio
+    async def test_a_write_carrying_no_extra_asks_nothing(self):
+        fake = FakeSpoolman()
+        client = _client(fake)
+
+        await client.create_spool(filament_id=7, remaining_weight=500.0)
+
+        assert fake.field_calls("tag") == []
+
+    @pytest.mark.asyncio
+    async def test_concurrent_syncs_ask_exactly_once_between_them(self):
+        """Two slots syncing at once must not race into a duplicate POST.
+
+        The exact call list, rather than just the POST count: the loser of the
+        race should re-read the memo once it holds the lock and find the answer
+        already there, rather than repeating the round-trip the winner just
+        made.
+        """
+        fake = FakeSpoolman()
+        client = _client(fake)
+
+        await asyncio.gather(
+            client.create_spool(filament_id=7, extra={"tag": json.dumps("A")}),
+            client.create_spool(filament_id=7, extra={"tag": json.dumps("B")}),
+        )
+
+        assert fake.field_calls("tag") == ["GET /field/spool/tag", "POST /field/spool/tag"]
+
+    @pytest.mark.asyncio
+    async def test_another_client_does_not_inherit_the_answer(self):
+        """The memo describes one Spoolman, so a re-pointed client starts over."""
+        fake = FakeSpoolman()
+        await _client(fake).create_spool(filament_id=7, extra={"tag": json.dumps("A")})
+
+        second_fake = FakeSpoolman()
+        await _client(second_fake).create_spool(filament_id=7, extra={"tag": json.dumps("B")})
+
+        assert "GET /field/spool/tag" in second_fake.log
+
+
+class TestEveryWritePathThatCarriesExtra:
+    @pytest.mark.asyncio
+    async def test_update_spool(self):
+        fake = FakeSpoolman()
+        client = _client(fake)
+        spool = await client.create_spool(filament_id=7)
+
+        updated = await client.update_spool(spool_id=spool["id"], extra={"tag": json.dumps("A")})
+
+        assert updated["extra"]["tag"] == json.dumps("A")
+
+    @pytest.mark.asyncio
+    async def test_update_spool_full(self):
+        fake = FakeSpoolman()
+        client = _client(fake)
+        spool = await client.create_spool(filament_id=7)
+
+        updated = await client.update_spool_full(spool_id=spool["id"], extra={"tag": json.dumps("A")})
+
+        assert updated["extra"]["tag"] == json.dumps("A")
+
+    @pytest.mark.asyncio
+    async def test_merge_spool_extra(self):
+        """The funnel behind linking and unlinking a tag from the inventory."""
+        fake = FakeSpoolman()
+        client = _client(fake)
+        spool = await client.create_spool(filament_id=7)
+
+        updated = await client.merge_spool_extra(spool["id"], {"tag": json.dumps("A")})
+
+        assert updated["extra"]["tag"] == json.dumps("A")
+
+    @pytest.mark.asyncio
+    async def test_a_key_no_registration_list_ever_named(self):
+        """``bambu_color_name`` is absent from the connect and startup lists.
+
+        It survives today only because two call sites remember to register it
+        by hand. Keying off the payload is what stops that being load-bearing.
+        """
+        fake = FakeSpoolman()
+        client = _client(fake)
+        spool = await client.create_spool(filament_id=7)
+
+        updated = await client.merge_spool_extra(spool["id"], {"bambu_color_name": json.dumps("Jade White")})
+
+        assert updated["extra"]["bambu_color_name"] == json.dumps("Jade White")
+        assert "bambu_color_name" in fake.registered
+
+    @pytest.mark.asyncio
+    async def test_every_key_of_a_multi_key_write(self):
+        fake = FakeSpoolman()
+        client = _client(fake)
+        spool = await client.create_spool(filament_id=7)
+
+        await client.merge_spool_extra(
+            spool["id"],
+            {"bambu_slicer_filament": json.dumps("GFA00"), "bambu_color_name": json.dumps("Black")},
+        )
+
+        assert {"bambu_slicer_filament", "bambu_color_name"} <= fake.registered
+
+
+class TestWhenRegistrationItselfFails:
+    @pytest.mark.asyncio
+    async def test_the_write_is_still_attempted(self):
+        """Best-effort: a failed registration must not swallow the write.
+
+        Spoolman still rejects the payload, exactly as it did before this
+        change -- the caller's error handling is what reports that, and it is
+        deliberately left untouched.
+        """
+        from backend.app.services.spoolman import SpoolmanClientError
+
+        fake = FakeSpoolman(field_status=500)
+        client = _client(fake)
+
+        with pytest.raises(SpoolmanClientError):
+            await client.create_spool(filament_id=7, extra={"tag": json.dumps("A")})
+
+        assert "POST /spool" in fake.log
+
+    @pytest.mark.asyncio
+    async def test_a_later_write_tries_registering_again(self):
+        """A failure is not cached -- Spoolman may simply have been restarting."""
+        from backend.app.services.spoolman import SpoolmanClientError
+
+        fake = FakeSpoolman(field_status=500)
+        client = _client(fake)
+        with pytest.raises(SpoolmanClientError):
+            await client.create_spool(filament_id=7, extra={"tag": json.dumps("A")})
+
+        fake.field_status = 200
+        result = await client.create_spool(filament_id=7, extra={"tag": json.dumps("B")})
+
+        assert result["extra"]["tag"] == json.dumps("B")

frontend/src/__tests__/components/SpoolmanSettings.test.tsx

@@ -200,7 +200,12 @@ describe('SpoolmanSettings', () => {
       });
     });
 
-    it('shows Connected and Disconnect button when connected', async () => {
+    it('shows Connected and offers nothing to press when connected', async () => {
+      // Spoolman is a stateless HTTP API with no session to close, so there is
+      // nothing for a Disconnect button to disconnect: it dropped this
+      // process's client object, which the next request rebuilt lazily, and the
+      // status flipped back on its own (#2903). Turning the integration off is
+      // the enable toggle's job.
       vi.mocked(api.getSpoolmanStatus).mockResolvedValue({
         enabled: true,
         connected: true,
@@ -211,8 +216,24 @@ describe('SpoolmanSettings', () => {
 
       await waitFor(() => {
         expect(screen.getByText('Connected')).toBeInTheDocument();
-        expect(screen.getByText('Disconnect')).toBeInTheDocument();
       });
+      expect(screen.queryByText('Disconnect')).not.toBeInTheDocument();
+      expect(screen.queryByText('Connect')).not.toBeInTheDocument();
+    });
+
+    it('offers Connect as a retry only while Spoolman is unreachable', async () => {
+      vi.mocked(api.getSpoolmanStatus).mockResolvedValue({
+        enabled: true,
+        connected: false,
+        url: 'http://localhost:7912',
+      });
+
+      render(<SpoolmanSettings />);
+
+      await waitFor(() => {
+        expect(screen.getByText('Connect')).toBeInTheDocument();
+      });
+      expect(screen.queryByText('Disconnect')).not.toBeInTheDocument();
     });
 
     it('shows sync section when connected', async () => {

frontend/src/components/SpoolmanSettings.tsx

@@ -1,7 +1,7 @@
 import { useState, useEffect } from 'react';
 import { useTranslation } from 'react-i18next';
 import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
-import { Loader2, Check, X, RefreshCw, Link2, Link2Off, Database, ChevronDown, Info, AlertTriangle, Package, ExternalLink } from 'lucide-react';
+import { Loader2, Check, X, RefreshCw, Link2, Database, ChevronDown, Info, AlertTriangle, Package, ExternalLink } from 'lucide-react';
 import { api, ApiError } from '../api/client';
 import type { SpoolmanSyncResult, Printer } from '../api/client';
 import { Card, CardContent, CardHeader } from './Card';
@@ -114,17 +114,6 @@ export function SpoolmanSettings() {
     },
   });
 
-  // Disconnect mutation
-  const disconnectMutation = useMutation({
-    mutationFn: api.disconnectSpoolman,
-    onSuccess: () => {
-      refetchStatus();
-    },
-    onError: () => {
-      showToast(t('settings.toast.saveFailed'), 'error');
-    },
-  });
-
   // Sync all mutation
   const syncAllMutation = useMutation({
     mutationFn: api.syncAllPrintersAms,
@@ -461,22 +450,14 @@ export function SpoolmanSettings() {
                     </span>
                   )}
                 </div>
+                {/* Retry affordance only. Spoolman is a stateless HTTP API with
+                    no session to hold open, so there is nothing for a Disconnect
+                    button to disconnect: it closed this process's client object,
+                    which any other request rebuilt lazily moments later, and the
+                    status above silently flipped back. The enable toggle is what
+                    turns the integration off. */}
                 <div className="flex gap-2">
-                  {status?.connected ? (
-                    <Button
-                      variant="secondary"
-                      size="sm"
-                      onClick={() => disconnectMutation.mutate()}
-                      disabled={disconnectMutation.isPending}
-                    >
-                      {disconnectMutation.isPending ? (
-                        <Loader2 className="w-4 h-4 animate-spin" />
-                      ) : (
-                        <Link2Off className="w-4 h-4" />
-                      )}
-                      {t('settings.disconnect')}
-                    </Button>
-                  ) : (
+                  {!status?.connected && (
                     <Button
                       size="sm"
                       onClick={() => connectMutation.mutate()}
@@ -494,9 +475,9 @@ export function SpoolmanSettings() {
               </div>
 
               {/* Error display */}
-              {(connectMutation.isError || disconnectMutation.isError) && (
+              {connectMutation.isError && (
                 <div className="mb-3 p-2 bg-red-100 dark:bg-red-500/20 border border-red-300 dark:border-red-500/50 rounded text-sm text-red-700 dark:text-red-400">
-                  {((connectMutation.error || disconnectMutation.error) as Error).message}
+                  {(connectMutation.error as Error).message}
                 </div>
               )}
 

static/index.html

@@ -26,7 +26,7 @@
 
     <!-- Splash screens for iOS -->
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
-    <script type="module" crossorigin src="/assets/index-DGi_kjAn.js"></script>
+    <script type="module" crossorigin src="/assets/index-FZchC7_S.js"></script>
     <link rel="stylesheet" crossorigin href="/assets/index-DynWy-72.css">
   </head>
   <body>