From 9c86a05657c70e7158be0b69c5113295b61c75a1 Mon Sep 17 00:00:00 2001 From: maziggy Date: Thu, 6 Aug 2026 12:21:50 +0200 Subject: [PATCH] Check filament deficit for Library-backed queue items (#2779) A job needing 20.5 g was dispatched onto a spool holding 9 g and the printer started. _resolve_source_3mf returned LibraryFile.file_path verbatim, but that column stores a path relative to base_dir -- so it resolved against the process working directory, found nothing, and compute_deficit_for_queue_item treated a missing source as "nothing to verify" and returned no deficit. Every library-backed queue item was affected: Slicer Pipeline jobs, which are always library-backed, and everything added through the Library's bulk Add to queue. Both callers share the resolver, so the Play button on the queue was as blind as the auto-dispatcher. Archive-backed items (print history, VP intake) resolved correctly and were never affected, and neither was PrintModal, which resolves the file on its own path. The library branch now uses the same idiom as the eleven other readers of file_path -- absolute stays, relative joins base_dir. The join carries a SEC-PATH-OK marker: the value is DB-stored and generated by the Library ingest, and it is already what resolves the file for upload, so the check has to resolve it identically or it is not checking what gets printed. A source that is configured but absent now logs a warning naming the item and the resolved path. It still dispatches, because the upload needs the same file seconds later and fails there, where blocking would strand a queue on a moved file -- but a safety check that skips itself must not do so in silence, which is what hid this for every library-backed item. Tests cover the relative path (the reporter's 20.5 g against 9 g), the absolute path against a base_dir the file is not under, and the missing-source warning. The existing cases all used archives with absolute paths, which is the gap the bug lived in. --- CHANGELOG.md | 1 + backend/app/services/filament_deficit.py | 35 +++++- .../unit/services/test_filament_deficit.py | 111 ++++++++++++++++++ 3 files changed, 144 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2af226be..b1ee9b0e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ All notable changes to Bambuddy will be documented in this file. - **Error and warning toasts now stay up twice as long** — Every pop-up notification disappeared after three seconds regardless of what it said. That is about right for "Settings saved", which confirms something you just did and is skimmed rather than read, but errors and warnings are a different kind of message: they carry a reason, often one relayed from the printer or the backend, and they run to a couple of lines. Three seconds was not long enough to finish reading one, and a missed error message is gone for good — there is no notification history to go back to. Errors and warnings now hold for six seconds. Success and informational toasts keep the three-second default, so the common case of clicking something and seeing it confirmed is unchanged, and the close button and the manual dismiss work exactly as before on all of them. The background print-dispatch toast is unaffected: it stays up while it has work in progress and clears itself shortly after the last job settles. Covered by frontend tests. ### Fixed +- **Prints queued from a Slicer Pipeline or the Library are checked for enough filament again (#2779, reported by @wylyn3d)** — A job needing 20.5 g was dispatched onto a spool holding 9 g, and the printer started. The same file, printed from the Print dialog, was correctly refused. The check that stands between the queue and the printer reads the sliced file to learn how much each slot needs, and it looked for that file in the wrong place: a file in the Library records where it lives relative to Bambuddy's data directory, and this one check read that as a path from wherever the process happened to be running. It found nothing, and a source it cannot find has always meant "nothing to verify" rather than "stop" — so the job passed a check that never actually ran. Every path that queues a Library file was affected: Slicer Pipeline jobs, which are always Library-backed, and anything added through the Library's **Add to queue**. Both the automatic dispatcher and the Play button on the queue were equally blind, so the deficit could not be caught by starting the job by hand either. Prints queued from print history were never affected, and neither was the Print dialog, which finds the file its own way. Two things changed: the check now resolves a Library file the same way the eleven other places that read one already did, and a source file that is configured but missing now writes a warning to the log naming the item and the path it looked at. That case still dispatches — the upload needs the same file moments later and fails there, where blocking would strand a queue on a file the user may have moved — but it no longer passes in silence, which is what let this go unnoticed. Covered by backend tests, including the reporter's exact 20.5 g against 9 g. - **A Forgejo token limited to a single repository can now be used for backups (#2775, reported by @AnthonyGrondin)** — Forgejo v15 lets you mint an access token that only reaches one repository, which is the safest token you can give a backup: leak it and the damage stops at the repository it was made for. Bambuddy refused it. **Test connection** asked Forgejo who the token belonged to before it asked whether the token could reach the repository, and a repository-scoped token is not allowed to answer that question — it may only carry read and write on issues and repositories — so the check failed on a token that would have backed up perfectly well. The identity question is now asked but no longer decides: only an outright rejection of the token is conclusive, and everything else falls through to the repository check, which is the one that matters. Nothing else in a backup ever needed the wider permission — the push writes through the repository's own contents endpoint and a restore reads its commits, trees and blobs — so ordinary tokens are unaffected. The message shown when the repository cannot be reached now names the scope to look for and the possibility that the token is scoped to a different repository, rather than only explaining Forgejo's habit of reporting a private repository as missing. The hint under the token field is also per provider now: it read "fine-grained token with Contents read/write" for all four, advice that only ever applied to GitHub, and now names GitHub's, GitLab's, Gitea's and Forgejo's own scopes in every language Bambuddy speaks. Covered by backend and frontend tests. - **Files queued from the Library are no longer missing from their owner's queue** — On an installation with authentication turned on, a user whose permissions are scoped to their own work saw an empty queue after adding files from the Library, and adding more only added more nothing. The jobs were really there and really printed; they simply belonged to no one. Every queue item records who created it, and the "own queue" permissions decide what to show by comparing that against the signed-in user — but the Library's bulk **Add to queue** never wrote it down, so its items matched no one and were visible only to users who can see the whole queue. This affected the one path built for adding many files at once, which is where it was hardest to notice something was wrong: the file list on screen looked no different afterwards. The same omission applied to the queue endpoint of the webhook API, whose items are now credited to the owner of the API key that added them. Items that genuinely have no one behind them are unchanged and still belong to no one — jobs sent through a virtual printer, anything added while authentication is off, and keys created before API keys had owners. Covered by backend tests. - **The drying popover no longer starts a cycle under a material you did not pick (#2774)** — An AMS-HT loaded with Support for PLA/PETG offered PLA in the drying dialog's filament list, and the cycle that started was labelled Support for PLA/PETG on the printer's own screen. Opening the dialog prefills it from the spool that is loaded, and Bambu reports that spool's material as `PLA-S` — a name Bambuddy's table of drying temperatures does not carry. The temperature and duration fell back to PLA's 45°C for twelve hours, which is what the dialog showed and what was sent, but the material did not fall back with them: it stayed `PLA-S`, and the dropdown, handed a value that is not one of its options, displays its first option without saying so. So the list read PLA while `PLA-S` was what left for the printer. Anyone who opened the dialog and pressed **Start** without touching the material was affected; picking any entry from the list, even the same one it was already showing, made the two agree again. The same gap covered every composite — a spool of PETG-CF, PLA-CF, ABS-GF or PAHT-CF prefilled the dialog at PLA's 45°C, far short of what those materials want, and sent its full name as the material. Bambuddy now resolves a spool's material to an entry the list actually has before either value is set, so what the dialog shows and what the printer is told can no longer disagree. Support materials and composites resolve to the material they are built on, so PLA-S dries as PLA and PETG-CF as PETG at 65°C, and nylon is recognised under the several spellings Bambu gives it. Anything genuinely unrecognised still falls back to PLA, deliberately the coolest setting in the table — under-drying an exotic filament costs a cycle, where defaulting to the hottest would deform a PLA spool. This does not address the other half of that report: a printer that keeps showing the material set from its own screen even when Bambuddy names a different one, which needs a capture of what Bambu Studio sends before anything can sensibly be changed. Covered by frontend tests. diff --git a/backend/app/services/filament_deficit.py b/backend/app/services/filament_deficit.py index 5271a3e5d..200f66ee4 100644 --- a/backend/app/services/filament_deficit.py +++ b/backend/app/services/filament_deficit.py @@ -79,11 +79,28 @@ def _global_to_ams_key(global_tray_id: int) -> tuple[int, int]: def _resolve_source_3mf(item: PrintQueueItem) -> Path | None: - """Locate the 3MF file backing this queue item (archive or library).""" + """Locate the 3MF file backing this queue item (archive or library). + + ``LibraryFile.file_path`` is stored relative to ``base_dir`` (rows written + before that convention hold absolute paths, which is why every reader + guards on ``is_absolute``). Resolving a relative one against the process + working directory finds nothing, and a source that cannot be found is + treated as "nothing to verify" — so this check silently passed every + library-backed item, which is every Slicer Pipeline job and everything + queued from the Library page (#2779). + """ if item.archive is not None and item.archive.file_path: return app_settings.base_dir / item.archive.file_path if item.library_file is not None and item.library_file.file_path: - return Path(item.library_file.file_path) + library_path = Path(item.library_file.file_path) + if library_path.is_absolute(): + return library_path + # SEC-PATH-OK: file_path is DB-stored and generated by the Library + # ingest (archive/library/files/.), never request input. The + # same value already resolves the file for upload in print_queue.py and + # print_scheduler.py — this check reads what the printer is about to be + # sent, so it must resolve it identically. + return app_settings.base_dir / item.library_file.file_path return None @@ -316,7 +333,19 @@ async def compute_deficit_for_queue_item( item = refreshed.scalar_one_or_none() or item source_path = _resolve_source_3mf(item) - if source_path is None or not source_path.exists(): + if source_path is None: + # No archive and no library file — nothing was ever attached to check. + return [] + if not source_path.exists(): + # Dispatch is not blocked: the upload that follows needs the same file + # and fails within seconds, where wedging the queue here would strand + # it. But skipping a safety check must leave a trace — a silent skip is + # what hid #2779 for every library-backed item. + logger.warning( + "Filament check skipped for queue item %s: source 3MF not found at %s", + item.id, + source_path, + ) return [] requirements = extract_filament_requirements(source_path, item.plate_id) diff --git a/backend/tests/unit/services/test_filament_deficit.py b/backend/tests/unit/services/test_filament_deficit.py index 51945785a..b168b6672 100644 --- a/backend/tests/unit/services/test_filament_deficit.py +++ b/backend/tests/unit/services/test_filament_deficit.py @@ -13,6 +13,7 @@ the contract for the cases that matter: from __future__ import annotations import json +import logging import zipfile from pathlib import Path from unittest.mock import patch @@ -63,6 +64,32 @@ async def _setup_archive_3mf(db_session, tmp_path: Path, filaments: list[dict]) return archive +async def _setup_library_3mf(db_session, base_dir: Path, filaments: list[dict], *, absolute: bool = False): + """Create a 3MF under ``base_dir`` and a LibraryFile row pointing at it. + + Mirrors production storage: the file lands in + ``/archive/library/files/`` and the row stores the path + *relative* to base_dir, exactly as ``library.py`` writes it (#2779). + """ + from backend.app.models.library import LibraryFile + + rel_path = Path("archive/library/files/deficit_probe.gcode.3mf") + abs_path = base_dir / rel_path + abs_path.parent.mkdir(parents=True, exist_ok=True) + _write_3mf(abs_path, filaments) + + lib_file = LibraryFile( + filename="deficit_probe.gcode.3mf", + file_path=str(abs_path) if absolute else str(rel_path), + file_type="3mf", + file_size=abs_path.stat().st_size, + ) + db_session.add(lib_file) + await db_session.commit() + await db_session.refresh(lib_file) + return lib_file + + async def _spool( db_session, *, @@ -103,10 +130,12 @@ async def _queue_item( archive: PrintArchive | None, ams_mapping: list[int] | None, plate_id: int | None = None, + library_file=None, ) -> PrintQueueItem: item = PrintQueueItem( printer_id=printer_id, archive_id=archive.id if archive else None, + library_file_id=library_file.id if library_file else None, ams_mapping=json.dumps(ams_mapping) if ams_mapping is not None else None, plate_id=plate_id, status="pending", @@ -226,6 +255,88 @@ class TestFilamentDeficit: assert deficit == [] + @pytest.mark.asyncio + async def test_library_file_with_relative_path_is_checked(self, db_session, printer_factory, tmp_path): + """#2779: a Library-backed item stores its path relative to base_dir. + + Resolving it against the process working directory finds nothing, and + "no source" is treated as "nothing to verify" — so the check returned + no deficit and the scheduler dispatched onto a spool that could not + finish the print. Every Slicer Pipeline item and everything queued via + the Library's Add to queue is library-backed, so the guard was absent + for all of them. Numbers are the reporter's: 20.5 g needed, 9 g left. + """ + printer = await printer_factory() + lib_file = await _setup_library_3mf( + db_session, + tmp_path, + [{"id": "1", "type": "PLA", "color": "#FFFFFF", "used_g": "20.5"}], + ) + assert not Path(lib_file.file_path).is_absolute() # the shape that broke + + spool = await _spool(db_session, label_weight=1000, weight_used=991.0) # 9g left + await _assign(db_session, printer_id=printer.id, spool_id=spool.id, ams_id=0, tray_id=0) + item = await _queue_item( + db_session, printer_id=printer.id, archive=None, library_file=lib_file, ams_mapping=[0] + ) + + with patch("backend.app.services.filament_deficit.app_settings.base_dir", tmp_path): + deficit = await compute_deficit_for_queue_item(db_session, item) + + assert len(deficit) == 1 + assert deficit[0].required_grams == 20.5 + assert deficit[0].remaining_grams == 9.0 + + @pytest.mark.asyncio + async def test_library_file_with_absolute_path_is_checked(self, db_session, printer_factory, tmp_path): + """The other half of the resolver: a row that already holds an absolute + path must not be joined onto base_dir a second time.""" + printer = await printer_factory() + lib_file = await _setup_library_3mf( + db_session, + tmp_path, + [{"id": "1", "type": "PLA", "color": "#FFFFFF", "used_g": "100.0"}], + absolute=True, + ) + spool = await _spool(db_session, label_weight=1000, weight_used=970.0) # 30g left + await _assign(db_session, printer_id=printer.id, spool_id=spool.id, ams_id=0, tray_id=0) + item = await _queue_item( + db_session, printer_id=printer.id, archive=None, library_file=lib_file, ams_mapping=[0] + ) + + # A base_dir the file is NOT under — joining it on would break the path. + with patch("backend.app.services.filament_deficit.app_settings.base_dir", tmp_path / "elsewhere"): + deficit = await compute_deficit_for_queue_item(db_session, item) + + assert len(deficit) == 1 + assert deficit[0].required_grams == 100.0 + + @pytest.mark.asyncio + async def test_missing_source_is_logged_not_just_skipped(self, db_session, printer_factory, caplog): + """A source that is configured but absent still dispatches — the upload + would fail seconds later anyway, and wedging the queue on a missing + file is the worse trade. But it must not pass silently: skipping the + check without a trace is what let #2779 go unnoticed for every + library-backed item. + """ + printer = await printer_factory() + archive = PrintArchive( + filename="ghost.3mf", + file_path="/nonexistent/ghost.3mf", + file_size=0, + status="completed", + ) + db_session.add(archive) + await db_session.commit() + await db_session.refresh(archive) + item = await _queue_item(db_session, printer_id=printer.id, archive=archive, ams_mapping=[0]) + + with caplog.at_level(logging.WARNING, logger="backend.app.services.filament_deficit"): + deficit = await compute_deficit_for_queue_item(db_session, item) + + assert deficit == [] + assert any("ghost.3mf" in r.getMessage() for r in caplog.records) + @pytest.mark.asyncio async def test_returns_empty_when_3mf_missing(self, db_session, printer_factory): printer = await printer_factory()