From cdc27eb517ad74980dd9696bb96a9eb51b2599f1 Mon Sep 17 00:00:00 2001 From: maziggy Date: Wed, 3 Jun 2026 09:07:57 +0200 Subject: [PATCH] =?UTF-8?q?=20=20fix(virtual-printer):=20#1429=20follow-up?= =?UTF-8?q?=20=E2=80=94=20auto-resolve=20VP=20IP=20when=20bind=5Faddress?= =?UTF-8?q?=20is=200.0.0.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original #1429 fix's _refresh_ip_encoding early-returned when mqtt_server.bind_address was "0.0.0.0" or empty (the default for VPs created without a dedicated bind IP). On a flat-LAN install that's the typical case, so the encoding never armed, _rewrite_net_info_ips was a no-op on every push, and the slicer kept following the real printer IP to its SD card. @Mape6 reported this on the 2026-06-02 daily that supposedly fixed the bug. New helper _resolve_host_interface_for_target() consults the existing network_utils.find_interface_for_ip() to pick the host interface in the printer's subnet. _refresh_ip_encoding falls back to it when bind_address is unspecified; an explicit bind IP still wins. INFO log line distinguishes the two paths ("armed: ... (bind_address)" vs "(auto-resolved)") so future bundles directly answer which IP the rewrite picked. Tests: 4 new under TestBindAddressAutoResolve — rewrite arms via auto-resolved IP at bind_address=0.0.0.0; stays disabled if no interface matches (no crash); explicit bind_ip still takes precedence; helper returns None defensively when find_interface_for_ip does. --- CHANGELOG.md | 1 + .../services/virtual_printer/mqtt_bridge.py | 49 +++++++- backend/tests/unit/test_vp_mqtt_bridge.py | 105 +++++++++++++++++- 3 files changed, 151 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4d8b34cd..728573708 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ All notable changes to Bambuddy will be documented in this file. ### Fixed - **Custom maintenance type "documentation URL" now persists on create (#1596, reported by @BurntOutHylian — with the exact root cause pre-triaged in the issue body)** — POST `/api/v1/maintenance/types` hard-coded every field on the `MaintenanceType` constructor by name (`name`, `description`, `default_interval_hours`, `interval_type`, `icon`, `is_system`) and silently dropped `wiki_url`, even though the Pydantic schema accepted it and the response model echoed it back as `null`. PATCH was fine because it used `data.model_dump(exclude_unset=True) + setattr`, which is why editing a freshly-created type DID save the URL — masking the bug under any "save then immediately fix it" test. **Fix**: add `wiki_url=data.wiki_url` to the constructor call at `routes/maintenance.py:206`. **Frontend nit also addressed in the same drop** (#1596 nit section): `MaintenancePage.tsx:1131` `updateTypeMutation`'s inline `Partial<{...}>` shape listed `name | default_interval_hours | interval_type | icon` only. The value reached the API correctly at runtime because `api.updateMaintenanceType` accepts `Partial` (which includes `wiki_url`), but the local type was misleading — anyone reading the mutation would wrongly conclude `wiki_url` wasn't part of the update payload. Extended the inline shape to include `wiki_url?: string | null`. **Tests**: one new integration test in `test_maintenance_api.py::test_create_custom_type_persists_wiki_url` — POSTs a custom type with a `wiki_url`, asserts the POST response carries it, and verifies via a separate GET round-trip that the value actually committed (defending against the "response echoes request body" failure mode the bug would have masked). Full 5565-test backend suite green; ruff clean; frontend build clean; ESLint zero output; touched MaintenancePage vitest green. - **External-folder `.gcode.3mf` files now show thumbnails, and every ingest path stores the same canonical `file_type` for sliced outputs (#1600, reported by @maziggy)** — Reporter noticed external-folder sliced outputs landed with no thumbnail. Cause: four backend ingest paths classified `LibraryFile.file_type` differently for the same `.gcode.3mf` family. The upload, ZIP-extract, and in-process paths used `os.path.splitext(filename)[1]` which returns `.3mf` for `foo.gcode.3mf`, stored `file_type="3mf"`, and matched the thumbnail-extraction gate at `library.py:1467` (`if file_type == "3mf":`). The external-folder scan path explicitly detected the compound and set `file_type="gcode.3mf"` — preserving the "sliced output" identity — but then skipped both `if file_type == "3mf":` (mismatch) and `if file_type == "gcode":` (also mismatch), so the file landed with `thumbnail_path = None`. Same compound-extension drift that bit #1543's 3D preview gates, just in a different surface that the #1543 frontend audit didn't trace back to. **Unified fix** (per the user's "unify if it's safe" directive): new `classify_file_type(filename)` helper in `library.py` is now the single source of truth — returns `gcode.3mf` for sliced outputs and `ext[1:]` otherwise. Applied to every ingest path: upload (`routes/library.py:1704`), ZIP-extract (`routes/library.py:1998`), external-folder scan (the bug site, plus the manual compound check is replaced), and the in-process `save_3mf_from_bytes()` helper (`routes/library.py:471` — used by MakerWorld import). The external-scan thumbnail gate is widened to `if file_type in ("3mf", "gcode.3mf"):` so a sliced output now goes through ThreeMFParser (a `.gcode.3mf` IS a 3MF zip with `Metadata/plate_1.png` thumbnail; the parser doesn't care about the trailing extension). The gcode-download endpoint at `GET /api/v1/library/files/{id}/gcode` (`routes/library.py:4390`) had the same drift in reverse — its gate was `elif file.file_type == "3mf":` so a row stored with `file_type="gcode.3mf"` (the external-scan path's pre-unification behaviour, and now the canonical going forward) was rejected with HTTP 400. Widened to `elif file.file_type in ("3mf", "gcode.3mf"):` so both ingest histories work. **One-shot DB migration** in `backend/app/core/database.py::run_migrations` backfills existing legacy rows: `UPDATE library_files SET file_type='gcode.3mf' WHERE file_type='3mf' AND LOWER(filename) LIKE '%.gcode.3mf'`. Idempotent (post-update rows no longer match the `file_type='3mf'` predicate, so re-runs at every boot are no-ops) and dialect-neutral (`LOWER` + `LIKE` are identical under SQLite and Postgres per the [[feedback_sqlite_and_postgres_upfront]] HARD RULE; behaviour-identical on Postgres by construction, tested explicitly on SQLite in the new regression suite). Without the backfill, users would have a permanent split state in the DB — old uploads at `3mf`, new uploads at `gcode.3mf` — which would (a) double-bucket sliced outputs in the dashboard stats query at `routes/library.py:4615` (`SELECT file_type, count(*) GROUP BY file_type`) and (b) show two entries in the file-manager filter dropdown for the same conceptual type. **Frontend untouched** — `FileManagerPage.tsx` and `ProjectDetailPage.tsx` already accept both `'3mf'` and `'gcode.3mf'` for Preview-3D, type-pill colour, and the file action gate per the #1543 fix. After the migration the DB only contains canonical values, so the legacy `'3mf'` branches in the frontend become dead code for sliced files — they stay in place to handle any future ingest path I missed (defence in depth — better a redundant gate than an empty card). **Tests**: 13 new in `test_library_classify_file_type.py` covering the helper across every compound / casing / no-extension case; 3 new in `test_library_file_type_backfill_migration.py` (legacy `.gcode.3mf`/`3mf` row backfilled, mixed-case filenames upgraded via `LOWER()`, unrelated `.bak`-suffixed compound substring left untouched, plain `.3mf` / raw `.gcode` / `.stl` untouched, idempotent on re-run); 2 new integration tests in `test_library_api.py` (upload of `.gcode.3mf` now stores `file_type="gcode.3mf"` via the unified path; the gcode-download endpoint accepts a row with `file_type="gcode.3mf"` and returns the embedded gcode). Full backend pytest 5564 passed under `-n 30`; ruff clean; frontend build clean; eslint zero output; i18n parity green at 5007 leaves × 9 locales. +- **Virtual-printer "Send file" IP rewrite now also fires for VPs without a dedicated bind IP (#1429 follow-up, residual case confirmed by @Mape6 on the 2026-06-02 daily)** — The first #1429 fix's `_refresh_ip_encoding` early-returned when `mqtt_server.bind_address` was `0.0.0.0` or empty (which is the default for any VP created without a bind IP selected — covered by the "Deferred (fix D)" note in the original #1429 changelog entry). On a flat-LAN install that's the typical case, so for those VPs the encoding never armed, `_rewrite_net_info_ips` was a no-op on every push, and the slicer kept following the real-printer IP to the printer's SD card — the exact symptom @Mape6 reported after pulling the 2026-06-02 daily that supposedly fixed this. **Fix (`backend/app/services/virtual_printer/mqtt_bridge.py`)**: new `_resolve_host_interface_for_target()` helper consults the existing `network_utils.find_interface_for_ip()` to pick the host interface in the same subnet as the printer's IP when `bind_address` is unspecified. `_refresh_ip_encoding` now falls back to that auto-resolved IP instead of returning early; an explicit bind IP still takes precedence. INFO log line distinguishes the two paths (`armed: ... (bind_address)` vs `armed: ... (auto-resolved)`) so support bundles answer "which IP did the rewrite pick?" without re-reasoning. If no interface matches the printer's subnet (the helper returns None), the bridge leaves encoding unarmed and the cache flows through as before — no crash, no wrong rewrite. **Tests** — 4 new in `backend/tests/unit/test_vp_mqtt_bridge.py::TestBindAddressAutoResolve`: rewrite arms via auto-resolved IP when bind_address is `0.0.0.0`; rewrite stays disabled when no host interface matches (no crash); explicit bind_ip takes precedence over auto-resolve; helper itself returns None when `find_interface_for_ip` does. All 39 mqtt_bridge tests pass; full backend unit suite (3667 tests) green; ruff clean. **Note on subnet matching**: the helper is best-effort — it picks the interface whose subnet contains the printer's IP, which is the right answer when slicer + printer + Bambuddy share a LAN (the typical home-lab case). Setups where the slicer reaches Bambuddy via a different interface than Bambuddy uses to reach the printer (multi-homed hosts, Tailscale + LAN where the slicer is on Tailscale and the printer on LAN) may still need an explicit bind IP — there's no leak in that case, just a rewritten value the slicer can't route to. The full audit-shaped resolution (enumerate accepted connections, per-slicer rewrite) is still a separate change. - **Virtual-printer "Send file" no longer redirects from Bambuddy to the physical printer's SD card once the printer powers on, and the mode button labels finally match the wire values stored in the DB (#1429, reported by @TrickShotMLG02, confirmed by @Mape6)** — Two reporters on completely different network topologies (3-subnet routed via OPNsense vs. flat single-LAN) saw the same symptom: with the physical printer off and Bambuddy freshly restarted, the slicer's "Send" landed in Bambuddy's archive; once the printer powered on, every subsequent "Send" went straight to the printer's SD card and bypassed Bambuddy entirely. @Mape6's packet capture on the flat-LAN case ruled out subnet / mDNS-reflector / firewall theories — the slicer just had a non-Bambuddy IP for the FTP destination once the printer was online. Bundle analysis: `mape6-before` (printer off) showed clean FTP receive + archive lines; `mape6-after` (printer on) had zero FTP connection attempts to Bambuddy, full stop. The mode-label discrepancy in every support bundle was a separate red herring that needed clearing up in the same drop. **Root cause** — `backend/app/services/virtual_printer/mqtt_bridge.py::_on_printer_raw` caches the real printer's `push_status` and rewrites `net.info[*].ip` from real-printer LE-uint32 to VP-bind-IP LE-uint32 so the slicer's FTP destination resolves to the VP. The rewrite has been in tree since 2026-05-03 and the unit test that ships with it passes. But the encoding (`_target_ip_uint32_le`, `_vp_ip_uint32_le`) was only computed inside `_resolve_client` on **client-identity change**, and `_resolve_client` early-returned (`if current is self._target_client: return`) on every refresh tick when the same client object was still bound. So if the printer's MQTT client object existed but `ip_address` was empty/stale at first bind (e.g. the printer's DB row hadn't picked up its discovered IP yet, or the client was constructed before the SSDP refresh), the encoded LE-uint32 stayed `None`, the rewrite block was skipped, the cache filled with the real printer IP, the sticky-keys preservation in the same function kept that poisoned `net` value alive across every subsequent incremental push, and the slicer followed the leaked IP to the real printer. The only way to clear it was to restart Bambuddy with the printer off — which is exactly the workaround both reporters independently arrived at. **Same shape on multi-NIC printers**: the rewrite only matched entries whose `ip` equalled `_target_ip_uint32_le`, so an X1C / H2D Pro reporting two active interfaces (WiFi + Ethernet) would have one entry rewritten and the other leaking the printer's other IP — a separate FTP fallback path that bypasses the VP even when the primary rewrite worked. **Fixes (mqtt_bridge.py)**: (1) `_resolve_client` now calls a new `_refresh_ip_encoding()` helper on every refresh tick, even when the client identity is unchanged — re-reads `current.ip_address`, re-encodes if either side changed, self-heals once `ip_address` becomes valid. (2) When the encoding becomes valid for the first time *after* the cache has already been populated, `_refresh_ip_encoding()` sweeps the cached `_latest_print_state` via the new `_rewrite_net_info_ips()` helper so the slicer's next pull sees the rewritten value — without this, sticky-key preservation keeps the poisoned cache alive across every incremental update. (3) `_rewrite_net_info_ips()` rewrites **every** non-zero `net.info[].ip` entry that doesn't already equal the VP bind IP, not only entries matching `_target_ip_uint32_le` — defensive against multi-NIC printers, against `_target_ip_uint32_le` being stale, and against unknown secondary interfaces leaking. Zero-IP entries (placeholders for unpopulated interfaces) are deliberately left alone so the slicer's "active interface" detection still recognises them as absent. (4) The rewrite path now logs at INFO when encoding arms or updates and at INFO when the cache sweep rewrites entries, so future support bundles directly answer "did the rewrite fire?" without re-reasoning about timing. **Mode wire-value rename (#1429 follow-up, separate confusion source)** — The UI button labeled "Archive" had always saved the wire value `immediate`, and "Queue" had always saved `print_queue`. Both reporters' support bundles showed `mode: immediate` while the UI said "Archive", and @TrickShotMLG02 specifically asked "I have no idea why it says immediate in the support-info.json file. In the webui the printer is set to archive". The mismatch was load-bearing for the debug session and had to be cleared up. Canonical wire values are now `archive` / `review` / `queue` / `proxy` matching the button labels 1:1. **Backend rename**: new `backend/app/models/virtual_printer.py::VP_MODE_*` constants + `normalize_vp_mode()` helper accepts legacy `immediate` / `print_queue` and translates to canonical. `VirtualPrinter.mode` default flipped to `archive`. `backend/app/services/virtual_printer/manager.py::VirtualPrinterInstance.__init__` normalises on construction so a legacy DB row read before the migration window has finished still dispatches to the correct handler; `on_file_received`, `on_print_command`, and `sync_from_db`'s change-detection all consume canonical values via `normalize_vp_mode()`. `backend/app/api/routes/virtual_printers.py::create_virtual_printer` and `update_virtual_printer` accept both forms on input and normalise to canonical before storage; `backend/app/api/routes/settings.py::get_virtual_printer_settings` normalises on read so frontend mode-button highlighting works for legacy stored values; `update_virtual_printer_settings` accepts and normalises on write. `backend/app/schemas/settings.py::AppSettings.virtual_printer_mode` default flipped to `archive` with updated description. **One-shot DB migration**: `backend/app/core/database.py::run_migrations` rewrites every `virtual_printers.mode` and `settings.virtual_printer_mode` row from `immediate` → `archive` and `print_queue` → `queue`. Idempotent — re-running on canonical values is a no-op, important because the full migration set runs every boot. Identical statement under SQLite and Postgres (plain `UPDATE ... WHERE` on a string column, no dialect-specific syntax) per the [[feedback_sqlite_and_postgres_upfront]] HARD RULE; tested explicitly on SQLite in the new regression suite, behaviour-identical on Postgres by construction. The historical single-VP migration (legacy `settings` rows → `virtual_printers` table on first multi-VP boot) gets the same `immediate` → `archive` / `print_queue` → `queue` translation; the historical `queue` → `review` alias is preserved because it predates the rename and reflected the user's intent at the time (the old wire `queue` meant "pending review", not "add to print queue"). **Frontend rename**: `VirtualPrinterSettings.tsx`, `VirtualPrinterCard.tsx`, and `VirtualPrinterAddDialog.tsx` all switched their button click handlers and `LocalMode`/`Mode` type aliases from `'immediate' | 'review' | 'print_queue' | 'proxy'` to `'archive' | 'review' | 'queue' | 'proxy'`. Each file gained its own `normalizeMode()` helper that translates legacy values arriving via stale-cached settings payloads to canonical, so the right mode button lights up even when the backend migration hasn't completed for that user's session yet. The two `printer.mode === 'queue' ? 'review' : printer.mode` legacy mappings in `VirtualPrinterCard.tsx::useEffect` and the error-recovery path have been replaced with `normalizeMode()` — they were the source of the test failure I caught mid-implementation where `mode: 'queue'` (the new canonical for the Queue button) was being incorrectly aliased back to `'review'` and hiding the auto-dispatch + force-color-match toggles. `frontend/src/api/client.ts::VirtualPrinterMode` is now the union of both canonical and legacy values (`'archive' | 'review' | 'queue' | 'proxy' | 'immediate' | 'print_queue'`) so older API clients (forks, mobile shortcuts, scripted setups) typecheck; the `updateSettings` body type narrows to canonical-only to steer new code. **Mode handler is NOT the dispatch bug**: `manager.py::_archive_file` is the handler for `archive` mode and it does archive-only (no dispatch to the physical printer). The user-visible "files end up on the printer's SD card" symptom was the IP-leak from the bridge cache, not a mode-dispatch bug. The mode rename is purely a clarity / support-bundle-accuracy fix. **Tests** — `backend/tests/unit/test_vp_mqtt_bridge.py`: 2 new in the bridge-rewrite class — `test_net_info_ip_rewritten_for_unknown_secondary_interface` covers the multi-NIC X1C / H2D Pro case where the printer reports an interface IP Bambuddy never saw; both entries get rewritten, the placeholder zero entry stays untouched. `test_late_arriving_printer_ip_rewrites_existing_cache` is the primary #1429 regression — bridge binds to a client with `ip_address=""`, first push lands and poisons the cache with the real-printer IP (the pre-fix state), the printer's `ip_address` then becomes known, the next `_resolve_client` tick arms the encoding AND sweeps the cached `net.info[].ip` so the slicer's next pull sees the VP IP. Without the sweep, sticky-key preservation would keep the poisoned value alive forever. `backend/tests/unit/test_vp_mode_rename_migration.py`: new file, 3 tests — legacy `immediate` → `archive` and `print_queue` → `queue` rewrites under SQLite, canonical values pass through untouched; legacy `virtual_printer_mode` setting also gets rewritten; running the migration twice is idempotent (every boot re-runs the full migration set). `backend/tests/integration/test_virtual_printer_api.py`: 3 reworked tests cover input-side normalisation — `test_update_mode_to_queue` asserts canonical, `test_update_mode_legacy_print_queue_normalises_to_queue` and `test_update_mode_legacy_immediate_normalises_to_archive` assert legacy → canonical translation on storage. The pre-existing `test_update_mode_legacy_queue_maps_to_review` (predating the rename, asserted the old `queue` → `review` alias) is removed; the new `test_update_mode_to_archive` covers canonical archive setting. **All other VP tests were updated to canonical** — `test_virtual_printer.py` (43 occurrences), `test_vp_diagnostic.py` (1), `test_virtual_printer_api.py` mocks (5) renamed; the `sync_from_db_restarts_on_mode_change` test had to be repaired by hand because the sed pass made both sides `archive` (defeating the change detection); now uses `archive` → `review` to actually exercise the change branch. Frontend: `VirtualPrinterCard.test.tsx`, `VirtualPrinterSettings.test.tsx`, `VirtualPrinterDiagnosticModal.test.tsx` updated to canonical fixtures and assertions; the legacy `queue maps to review` test in `VirtualPrinterSettings.test.tsx` replaced with two tests — legacy `immediate` lights up the Archive button, legacy `print_queue` lights up the Queue button, both via the new client-side `normalizeMode()` helper. The five `InventoryPage*.test.tsx` files that hardcoded `virtual_printer_mode: 'immediate'` in their settings mocks bulk-renamed to `'archive'`. **CI gates green**: backend pytest 5546 passed in 73.88s + 7.10s under `-n 30` / `-n 12` parallel; ruff clean; frontend `npm run build` clean (TypeScript + Vite); ESLint zero output; vitest 2045 passed in 26.12s; i18n parity script clean at 5007 leaves × 9 locales. **Deferred (fix D in the diagnosis writeup)**: bind_ip == 0.0.0.0 path. The rewrite is still explicitly skipped when bind_ip is the unspecified address, which is correct for the routing (you can't tell a slicer to FTP to 0.0.0.0) but leaves users without a dedicated bind IP exposed to the same IP-leak pattern. Both reporters had `has_bind_ip=true` in their bundles so this isn't load-bearing for #1429 itself; will be addressed as a separate audit-shaped change that needs to enumerate the host's outbound IPs and pick the one that can reach the printer, with its own test surface. **Out of scope for this PR**: port 40024 in @Mape6's packet capture (Bambu Network Plugin's LAN-Send pre-flight port) — a probe that arrives at the VP IP, finds no listener, and the slicer falls back. Adding a 40024 listener is conceptually a different surface (handshake parsing, not MQTT cache state) and the cache-leak fix alone removes the underlying redirection so the 40024 probe lands on a VP that's actually the right destination. Will reassess if either reporter still sees mis-routing after this fix. - **Multi-plate `.gcode.3mf` archives + reprints no longer under-report filament, time, and cost — project stats and parser both fixed (#1593, reported by @needo37)** — Reporter printed 3 plates of a multi-plate file: Archive Print Log correctly recorded 3 completed runs at distinct durations and filament weights; Project page showed `Print Jobs: 1 / 1 parts printed`, plate-1's `1h53m / 58g / $1.09`; Archive card said `3 prints` but rendered plate-1's `57.6g / 1h45m / 1 object`. Two distinct causes stacked. **Root cause 1 — 3MF parser only read the first plate**: `ThreeMFParser._parse_slice_info` (`backend/app/services/archive.py:191`) called `root.find(".//plate")` and pulled `prediction` / `weight` from that one element — so for any multi-plate file the archive's file-level `print_time_seconds` / `filament_used_grams` reflected plate 1 alone. The per-plate `/plates` endpoint already looped `findall(".//plate")` and was correct, which is why the plate carousel showed the right numbers while the archive card was wrong. **Root cause 2 — project rollup aggregated `PrintArchive`, not the per-run log**: `compute_project_stats` and the `list_projects` quick-stats block (`backend/app/api/routes/projects.py`) summed `PrintArchive.print_time_seconds / filament_used_grams / cost / energy_*` `WHERE project_id = X`. A reprint reuses the source archive row and only adds a new `PrintLogEntry`, so 3 sequential runs of one file collapsed to 1 archive — and that archive's numbers were already plate-1-only because of root cause 1. The Archive Print Log path was correct because it already drove off `print_log_entries` (`archives.py:420` — *"Reads from print_log_entries so reprints contribute each run"*); project stats just hadn't been pointed at the same source. **Parser fix**: `_parse_slice_info` now loops `findall(".//plate")` and sums `prediction` → `print_time_seconds` and `weight` → `filament_used_grams` across all plates. Per-plate concepts (`plate_number`, `_plate_index`, `printable_objects`) are only set when there's exactly one plate — for multi-plate exports the archive represents all plates and a single plate index is meaningless at the file level. `bed_type` keeps the first plate's value as a best-effort archive default. Malformed `prediction` / `weight` values on individual plates skip cleanly rather than poison the sum. **Stats fix**: `compute_project_stats` and the `list_projects` quick-stats block both switch to an inner join `print_log_entries → print_archives` `WHERE archives.project_id = X`. `total_archives` becomes `COUNT(PrintLogEntry.id)` (actual runs, not files); `failed_prints` becomes the count of runs in `failed/aborted/cancelled/stopped`; `completed_items` becomes `SUM(PrintArchive.quantity)` filtered to runs with `status='completed'` (each run contributes its archive's quantity); `total_print_time_hours / total_filament_grams / estimated_cost / total_energy_*` come from `PrintLogEntry` columns. Orphan log rows (`archive_id IS NULL` after archive deletion via `ON DELETE SET NULL`) are excluded by the inner join — they can't be attributed to any project. **Backfill behaviour** (intentional, matches the reporter's "forward-only" note): users with AMS spool tracking — the reporter's case — have per-run `PrintLogEntry.filament_used_grams` from the tracked spool delta, not the plate-1 estimate, so project stats become correct *immediately* after the rollup fix with no reslice required. Users without tracking fall back to the archive estimate; their stats undercount until they reprint with the fixed parser. The Archive **card** still reads `PrintArchive.filament_used_grams` directly, so old archives keep their plate-1-only numbers until a reslice/rescan repopulates `file_metadata`. **Same-shape fix carried forward**: `system.py::system_info` (the System Info page's lifetime totals) summed `PrintArchive.print_time_seconds` / `filament_used_grams` with the identical bug — reprints collapsed to one archive, multi-plate files reported plate-1-only. The route now sums from `PrintLogEntry.duration_seconds` / `filament_used_grams` like the project rollup, so every run contributes its measured per-run actual. **Same-shape fix in the time-accuracy metric** (`archives.py::get_archive_stats`): the metric computed `estimate / actual` per run where `estimate = PrintArchive.print_time_seconds`. Post-parser-fix multi-plate archives have file-level estimate but per-run actual = one plate's duration → ratio ≈ N×100% for an N-plate file (300% for the reporter's 3-plate case), which would drag the printer-level average to noise. The calc now clamps each row to the [50%, 200%] plausibility band before contributing to the average; single-plate accuracy is fully included (the case the metric is designed for), multi-plate plate-by-plate runs and one-off outliers (manual intervention, purge waste blowing the estimate) are excluded. **Tests**: 4 new in `test_archive_service.py::TestMultiPlateSliceInfoSum` — three-plate file sums prediction + weight (the reporter's exact numerics: 7140+6000+6300 → 19440s, 19.2+20.0+18.8 → 58.0g); single-plate path preserves `plate_number` + objects + bed_type; multi-plate ignores per-plate object lists; malformed per-plate values are skipped without poisoning the sum. 4 new in `test_projects_api.py::TestProjectStatsPerRun` — 3 reprints show as 3 jobs with summed totals (matches the reporter's exact 3-run scenario); orphan log entries don't bleed into any project; mixed-outcome archive splits cleanly between `completed_prints` (quantity-weighted) and `failed_prints` (run-counted); list-view quick stats agree with per-project stats. 1 new in `test_archive_run_aggregation.py` — the accuracy band filter excludes multi-plate plate-by-plate runs (estimate 18000s / actual 6000s = 300%) so a single-plate file's near-100% reading stays the printer's average. Two pre-existing assertions updated to reflect the corrected semantics: `archive_count` and `total_archives` now count runs, so files attached but never printed (status `"archived"`) contribute 0 — that's the right answer, not a regression. Full backend suite + ruff clean. - **Webhook printer-status / stop / cancel routes 500'd on every connected printer because the route treated the PrinterState dataclass as a dict (#1584, reported via in-app bug report)** — Reporter saw `GET /api/v1/webhook/printer/{id}/status` return `500 Internal Server Error` with a valid API key carrying the `read_status` scope, while `GET /api/v1/system/info` returned 200 with the same key — so auth and routing were fine, the handler itself was crashing. Cause: `printer_manager.get_status(printer_id)` returns a `PrinterState` dataclass (`backend/app/services/bambu_mqtt.py`), not a dict. The route at `webhook.py:266-270` called `status.get("connected", False)`, `status.get("state")`, `status.get("current_print")`, `status.get("progress")`, `status.get("remaining_time")` — every one raised `AttributeError`, which Starlette surfaced as a generic 500. Reporter's id-1 (printer exists) returned 500; non-existent ids returned 404 — exactly because the early `Printer not found` branch fired before reaching the crash. Same shape in two adjacent routes: `webhook_stop_print` (`POST /printer/{id}/stop`) and `webhook_cancel_print` (`POST /printer/{id}/cancel`) checked `status.get("connected")` / `status.get("state")` for their precondition gates. 8 crash sites total across the three routes. **Fix**: every `status.get("X", default)` replaced with attribute access (`status.X if status else default`); Pydantic response schema unchanged. `PrinterState`'s dataclass defaults cleanly cover the `status is None` branch (printer registered but never connected — the route now returns 200 with `connected=false, state=null, …` rather than crashing). **Tests** (`backend/tests/integration/test_webhook_printer_status.py`): 7 new — status route returns 200 with the dataclass attributes mapped into the response (regression for the exact #1584 shape); status route returns 200 with sensible defaults when `get_status()` returns None; status route returns 404 for a non-existent printer (control case proving the auth path is unaffected); stop route returns 503 when disconnected (pre-fix would have 500'd here); stop route returns 409 when state is not `RUNNING`; cancel route returns 503 when disconnected; cancel route returns 409 when state is not `RUNNING`/`PAUSE`. Runtime-verified end-to-end against a live PG-backed instance before and after: same key + same printer id, 500 before the patch and 200 with the correct payload after. Full backend suite + ruff clean. diff --git a/backend/app/services/virtual_printer/mqtt_bridge.py b/backend/app/services/virtual_printer/mqtt_bridge.py index a95a0b245..65bbdff1a 100644 --- a/backend/app/services/virtual_printer/mqtt_bridge.py +++ b/backend/app/services/virtual_printer/mqtt_bridge.py @@ -90,6 +90,33 @@ def _ip_to_uint32_le(ip_str: str) -> int: return parts[0] | (parts[1] << 8) | (parts[2] << 16) | (parts[3] << 24) +def _resolve_host_interface_for_target(target_ip: str) -> str | None: + """Pick a host-side IPv4 for `net.info[].ip` when the VP has no dedicated bind IP. + + Used when `mqtt_server.bind_address` is empty or 0.0.0.0 — the listener + accepts on every interface but we still need ONE concrete IPv4 to write + into the rewritten `net.info[].ip` field so the slicer's FTP target + resolves to Bambuddy rather than the real printer. Returns the IPv4 of + the host interface that shares a subnet with the printer (best fit + because the slicer is typically on the same LAN as the printer), or + None if no interface matches — in which case the bridge leaves + encoding unarmed and the previous (still-leaky) behaviour stands. + """ + try: + from backend.app.services.network_utils import find_interface_for_ip + except Exception: # pragma: no cover - import shielding + return None + try: + iface = find_interface_for_ip(target_ip) + except Exception: + logger.exception("MQTT bridge: find_interface_for_ip(%s) crashed", target_ip) + return None + if not iface: + return None + ip = iface.get("ip") + return ip if isinstance(ip, str) and ip else None + + def _merge_ams_dict(prev_ams: dict, new_ams: dict) -> dict: """Merge a new ``ams`` blob from an incremental push onto the previous one. @@ -354,16 +381,31 @@ class MQTTBridge: printer IP, also sweep the existing cache so the slicer's next pull sees the rewritten value (#1429). Without this sweep the sticky-key preservation keeps the poisoned `net.info[].ip` alive forever. + + VP bind IP resolution: when `mqtt_server.bind_address` is empty or + `0.0.0.0` (the default for VPs that were never assigned a dedicated + bind IP), fall back to auto-resolving the host interface in the same + subnet as the printer's IP. Without this fallback, the rewrite never + arms on a default-config flat-LAN install and `net.info[].ip` leaks + the real printer IP — slicer follows it on Send (#1429 residual). """ client = self._target_client if client is None: return target_ip = getattr(client, "ip_address", None) - vp_ip = getattr(self._mqtt_server, "bind_address", None) - if not target_ip or not vp_ip or vp_ip in ("0.0.0.0", "", None): # nosec B104 + if not target_ip: return + vp_ip = getattr(self._mqtt_server, "bind_address", None) + vp_ip_source = "bind_address" + if not vp_ip or vp_ip in ("0.0.0.0", ""): # nosec B104 + resolved = _resolve_host_interface_for_target(target_ip) + if not resolved: + return + vp_ip = resolved + vp_ip_source = "auto-resolved" + try: new_target_le = _ip_to_uint32_le(target_ip) new_vp_le = _ip_to_uint32_le(vp_ip) @@ -379,11 +421,12 @@ class MQTTBridge: self._target_ip_uint32_le = new_target_le self._vp_ip_uint32_le = new_vp_le logger.info( - "[%s] MQTT bridge IP encoding %s: target=%s vp=%s", + "[%s] MQTT bridge IP encoding %s: target=%s vp=%s (%s)", self.vp_name, "updated" if was_armed else "armed", target_ip, vp_ip, + vp_ip_source, ) cached = self._latest_print_state diff --git a/backend/tests/unit/test_vp_mqtt_bridge.py b/backend/tests/unit/test_vp_mqtt_bridge.py index 28d81a50a..7515e30ce 100644 --- a/backend/tests/unit/test_vp_mqtt_bridge.py +++ b/backend/tests/unit/test_vp_mqtt_bridge.py @@ -7,7 +7,11 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest -from backend.app.services.virtual_printer.mqtt_bridge import MQTTBridge, _ip_to_uint32_le +from backend.app.services.virtual_printer.mqtt_bridge import ( + MQTTBridge, + _ip_to_uint32_le, + _resolve_host_interface_for_target, +) from backend.app.services.virtual_printer.mqtt_server import SimpleMQTTServer H2D_SERIAL = "0948BB540200427" @@ -1081,3 +1085,102 @@ class TestIpEncoding: def test_invalid_ip_raises(self): with pytest.raises(ValueError): _ip_to_uint32_le("not.an.ip.actually") + + +# --------------------------------------------------------------------------- +# Auto-resolve fallback for default-config (bind_address = "0.0.0.0") +# --------------------------------------------------------------------------- + + +class TestBindAddressAutoResolve: + """#1429 residual: VPs created without a dedicated bind IP run on + `bind_address=0.0.0.0`. The original fix's `_refresh_ip_encoding` + early-returned on 0.0.0.0, so the rewrite never armed and `net.info[].ip` + kept leaking the real printer IP. Now the bridge auto-resolves a host + interface in the printer's subnet and uses that as the VP IP.""" + + @pytest.mark.asyncio + async def test_rewrite_arms_via_auto_resolved_host_ip(self): + """When bind_address is 0.0.0.0, fall back to the host interface in + the target printer's subnet and rewrite to that IP.""" + server = _make_server(bind_address="0.0.0.0") + bridge = _make_bridge(server) + with patch( + "backend.app.services.virtual_printer.mqtt_bridge._resolve_host_interface_for_target", + return_value=VP_IP, + ): + await bridge.start() + + h2d_le = _ip_to_uint32_le(H2D_IP) + vp_le = _ip_to_uint32_le(VP_IP) + payload = json.dumps( + { + "print": { + "command": "push_status", + "net": {"info": [{"ip": h2d_le, "mask": 0xFFFFFF}]}, + } + } + ).encode() + bridge._on_printer_raw(f"device/{H2D_SERIAL}/report", payload) + await asyncio.sleep(0.01) + + cached = bridge.get_latest_print_state() + assert cached["net"]["info"][0]["ip"] == vp_le + assert bridge._vp_ip_uint32_le == vp_le + + await bridge.stop() + + @pytest.mark.asyncio + async def test_rewrite_disabled_when_no_matching_host_interface(self): + """If no host interface shares a subnet with the printer, the bridge + cannot pick a sensible VP IP — leave encoding unarmed and let the + push through unrewritten (no crash, no wrong rewrite).""" + server = _make_server(bind_address="") + bridge = _make_bridge(server) + with patch( + "backend.app.services.virtual_printer.mqtt_bridge._resolve_host_interface_for_target", + return_value=None, + ): + await bridge.start() + + h2d_le = _ip_to_uint32_le(H2D_IP) + payload = json.dumps( + { + "print": { + "command": "push_status", + "net": {"info": [{"ip": h2d_le, "mask": 0xFFFFFF}]}, + } + } + ).encode() + bridge._on_printer_raw(f"device/{H2D_SERIAL}/report", payload) + await asyncio.sleep(0.01) + + assert bridge._vp_ip_uint32_le is None + assert bridge._target_ip_uint32_le is None + + await bridge.stop() + + @pytest.mark.asyncio + async def test_explicit_bind_ip_takes_precedence_over_auto_resolve(self): + """Auto-resolve only kicks in when bind_address is empty/0.0.0.0; an + explicitly-set bind IP must be used verbatim even if there's also a + same-subnet host interface.""" + server = _make_server(bind_address=VP_IP) + bridge = _make_bridge(server) + # Auto-resolver would have returned a DIFFERENT IP — we must not use it. + with patch( + "backend.app.services.virtual_printer.mqtt_bridge._resolve_host_interface_for_target", + return_value="10.99.99.99", + ): + await bridge.start() + assert bridge._vp_ip_uint32_le == _ip_to_uint32_le(VP_IP) + await bridge.stop() + + def test_resolve_helper_returns_none_for_unreachable_target(self): + """The helper itself must be defensive — if `find_interface_for_ip` + raises or returns None, we get None (no crash).""" + with patch( + "backend.app.services.network_utils.find_interface_for_ip", + return_value=None, + ): + assert _resolve_host_interface_for_target("203.0.113.1") is None