diff --git a/CHANGELOG.md b/CHANGELOG.md index ef72eed86..b4d8b34cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ All notable changes to Bambuddy will be documented in this file. ### Security - **Path-traversal hardening across the upload / import / file-write surface (routes + services); fifth CI backstop ships alongside** — A private path-traversal report against `POST /api/v1/projects/import/file` traced two attacker-controlled strings being joined to `library_dir` with no resolve + containment check: (a) `linked_folders[*].name` from the request's `project.json` ("Vector A" — an absolute path in this field collapsed `library_dir / "/anywhere"` to `Path("/anywhere")` because pathlib discards the left side when the right is absolute, letting the next `write_bytes` land anywhere the backend could write), and (b) per-entry `zf.namelist()` paths from the ZIP itself ("Vector B" — ZIP filenames carry `..` segments by spec and the join `library_dir / folder_name / relative_path` had no per-component check). Concrete escalation: drop a `.pth` file into the venv's `site-packages` directory for code execution on next service restart; overwrite the JWT signing-secret file to forge an admin token; overwrite `~/.ssh/authorized_keys` or `~/.bashrc` on native installs. **Fix is structural, not just patch the diff** (per [[feedback_dont_dismiss_preexisting]]). New `backend/app/utils/safe_path.py::safe_join_under(parent, *parts)` helper joins under a trusted parent, resolves both sides, asserts `is_relative_to(parent.resolve())`, and rejects up-front empty / null-byte / absolute path components. Wired into `import_project_file` at both vectors. **Adjacent fix from the routes audit**: `GET /api/v1/archives/{id}/photos/{filename}` had NO validation on `filename` and FileResponse-served arbitrary paths — the existing DELETE endpoint at least had a membership check against `archive.photos` (which is UUID-generated on upload), but GET shared neither the check nor any traversal guard. Both GET and DELETE now route through `safe_join_under` for defence-in-depth on top of the membership check. **Second adjacent fix from the services audit**: `ArchiveService.attach_timelapse(archive_id, data, filename)` in `backend/app/services/archive.py:1456` wrote `archive_dir / filename` where `filename` ultimately comes from either a printer's FTP listing (compromised-printer threat model — the printer is part of the trust surface) or the `?filename=...` query param on `POST /api/v1/archives/{id}/timelapse/select`. A malicious printer that returns a directory listing entry with `..` segments could write the timelapse bytes outside the archive directory; the `f.get("name") == filename` gate in the route did not prevent it because the gate is satisfied by whatever the printer claims is on disk. `attach_timelapse` now routes through `safe_join_under(..., http=False)` and returns `False` (logging the rejection) when the join would escape — matching the existing not-found contract of the function rather than raising 400 from inside a background task. **Audit sweep methodology**: AST-walked every Python file under `backend/app/api/routes/` AND `backend/app/services/` for `Path / Name` shapes (the exact shape that produced the original report). 25 additional route-layer sites and 8 additional service-layer sites confirmed safe case-by-case (UUID-generated filenames written by Bambuddy itself, `_safe_filename(...)` / `Path(arg).name` basename-stripped inputs, `os.walk`-discovered names, denylist + format-validated backup names, hardcoded constants iterated through a tuple, DB-stored paths whose write origin already goes through a resolved-and-containment-checked helper). Each safe site got a `# SEC-PATH-OK: ` marker so future audits can trust the inline guard at a glance. Six pre-existing safe-with-marker sites (`library.py` external upload, `archives.py` timelapse output, `projects.py` attachment download/delete, `settings.py` backup extractall) carry the same marker shape. **Fifth CI backstop** `test_route_path_arithmetic_is_safe_joined_or_marked` (`backend/tests/unit/test_no_unsafe_path_joins.py`) AST-walks every Python file in `backend/app/api/routes/` AND `backend/app/services/` and fails the build on any ` / ` join that doesn't either route through `safe_join_under` or carry the marker on the join line. Joins matching the higher-structure shapes (Attribute access, Subscript, f-string, `str(...)` call) are categorically different and out of scope — those are caught by the broader audit sweep, not the regression backstop. The services layer is in scope because it receives values from the routes verbatim AND from external sources Bambuddy has no control over (the printer FTP-listing case above). **Tests**: 17 unit tests for `safe_join_under` covering every escape vector (absolute path, Windows abs path, `..` segments, embedded `..`, null byte, empty string, no parts, non-str, plus legitimate nested-path round-trip); 4 integration tests against `POST /api/v1/projects/import/file` exercising the full FastAPI stack with the verbatim shape from the report (absolute path in `folder_name` → 400 + filesystem assertion that the target file doesn't exist; `..` in `folder_name` → 400; `..` in `relative_path` → 400; legitimate nested ZIP still imports cleanly to guard against the fix being over-strict); 3 unit tests against `ArchiveService.attach_timelapse` exercising the compromised-printer threat model (filename with `..` segments → returns False + no file at the escape target; absolute filename → returns False + no file at `/tmp`; legitimate `timelapse_YYYY-MM-DD_HH-MM-SS.mp4` → returns True + file lands inside archive_dir, guarding against the fix being over-strict). **SECURITY.md** gains a fifth rule + a fifth row in the CI-test mapping table; the rule explicitly names the printer FTP-listing case as in-scope to set the expectation for future services-layer audits. Full 5500+ test backend suite green; ruff clean. ### 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" 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. diff --git a/backend/app/api/routes/maintenance.py b/backend/app/api/routes/maintenance.py index 9c400b8aa..8ed1e65fd 100644 --- a/backend/app/api/routes/maintenance.py +++ b/backend/app/api/routes/maintenance.py @@ -209,6 +209,7 @@ async def create_maintenance_type( default_interval_hours=data.default_interval_hours, interval_type=data.interval_type, icon=data.icon, + wiki_url=data.wiki_url, is_system=False, ) db.add(new_type) diff --git a/backend/tests/integration/test_maintenance_api.py b/backend/tests/integration/test_maintenance_api.py index 8894aa776..7c4e4b171 100644 --- a/backend/tests/integration/test_maintenance_api.py +++ b/backend/tests/integration/test_maintenance_api.py @@ -46,6 +46,31 @@ class TestMaintenanceTypesAPI: assert result["name"] == "Custom Test Task" assert result["is_system"] is False + @pytest.mark.asyncio + @pytest.mark.integration + async def test_create_custom_type_persists_wiki_url(self, async_client: AsyncClient): + """#1596: pre-fix, the POST handler hard-coded every constructor field + by name and silently dropped `wiki_url`. The schema accepted the value, + the response echoed `null`, and the row landed without it. Pin the + contract so the constructor doesn't drift again.""" + data = { + "name": "Wiki URL Persistence Test", + "default_interval_hours": 50.0, + "interval_type": "hours", + "wiki_url": "https://wiki.example.com/lubrication", + } + response = await async_client.post("/api/v1/maintenance/types", json=data) + assert response.status_code == 200 + assert response.json()["wiki_url"] == "https://wiki.example.com/lubrication" + + # Verify it persists through a separate GET round-trip — the POST + # response could have echoed the request body without committing. + list_response = await async_client.get("/api/v1/maintenance/types") + assert list_response.status_code == 200 + matching = [t for t in list_response.json() if t["name"] == data["name"]] + assert len(matching) == 1 + assert matching[0]["wiki_url"] == "https://wiki.example.com/lubrication" + @pytest.mark.asyncio @pytest.mark.integration async def test_update_maintenance_type(self, async_client: AsyncClient): diff --git a/frontend/src/pages/MaintenancePage.tsx b/frontend/src/pages/MaintenancePage.tsx index c34eb4697..ae7737730 100644 --- a/frontend/src/pages/MaintenancePage.tsx +++ b/frontend/src/pages/MaintenancePage.tsx @@ -1128,7 +1128,11 @@ export function MaintenancePage() { // directly in onAddType callback const updateTypeMutation = useMutation({ - mutationFn: ({ id, data }: { id: number; data: Partial<{ name: string; default_interval_hours: number; interval_type: 'hours' | 'days'; icon: string }> }) => + // `wiki_url` is part of `MaintenanceTypeCreate` and reaches the API + // correctly at runtime (the api helper takes `Partial`), + // but the inline shape on this mutation used to omit it — making the + // type lie about what the payload carries (#1596 nit). + mutationFn: ({ id, data }: { id: number; data: Partial<{ name: string; default_interval_hours: number; interval_type: 'hours' | 'days'; icon: string; wiki_url: string | null }> }) => api.updateMaintenanceType(id, data), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ['maintenanceTypes'] });