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.

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>