The drying popover prefilled its material from the loaded spool without
checking the preset table had that material. An AMS-HT holding Support for
PLA/PETG (tray_type PLA-S) fell back to PLA's temperature but kept PLA-S as
the material, and the dropdown displays its first option when handed a value
outside its list -- so it read PLA while PLA-S was sent. Same gap for every
composite: PETG-CF prefilled at PLA's 45C.
Resolve the tray_type to a key the table has before setting either value.
Support materials and composites resolve to their base, nylon is aliased
under its several spellings, and anything unrecognised falls back to PLA --
the coolest row, so an unknown material under-dries rather than deforming a
PLA spool.
Also record request-topic messages in the MQTT debug log. That topic carries
every command a printer is given, including Bambu Studio's, and returned
before the logging block -- so a capture could show only what the printer
said, never what it was told.
A settings restore could substitute the instance'"'"'s authentication source.
auth.py reads the LDAP config live from the settings table on every
login, and none of ldap_server_url, ldap_user_filter, ldap_auto_provision
or ldap_default_group is credential-shaped, so the secret-key hints never
saw them and only the four auth-policy keys were protected.
ldap_enabled was covered by the companion-credential rule instead, and
that rule asks the wrong question. It judges availability - "will the
integration still work?" - and an anonymous bind works, so a payload that
simply OMITS ldap_bind_password skips the refusal and has its toggle
written. Omitting the credential is exactly what an attacker authoring
the file would do: they own the directory being pointed at, so they need
no bind credential from us.
Left unrefused, a backup repository anyone can write to yields admin:
point ldap_server_url at your own directory, set ldap_auto_provision and
ldap_default_group=Administrators, and the next login on a fresh username
is provisioned into the admin group. Overwrite-off is enough on an
instance that never configured LDAP - there are no rows to skip.
Refused by prefix so a key added to the LDAP schema later is refused by
default, and matched case-insensitively because the key comes from the
backup JSON rather than from our own writer. ldap_enabled leaves
_COMPANION_CREDENTIALS rather than sitting there as dead code, since
_is_protected_setting_key runs first.
The two tests asserting an anonymous bind was a false positive are
inverted - they encoded the hole - and the refusal reuses the existing
settingsAuthSkipped note, which already points at Settings >
Authentication.
The branch had never added one, though the convention here is one bullet
per PR in the same commit as the code.
One entry for the whole feature rather than one per commit - it has not
shipped yet, so the review rounds are refinements of an unreleased thing
rather than fixes to a released one. For the same reason the per-category
permissions are written as what to grant, not as something that changed.
One build on the tip after merging dev, per the branch'"'"'s standing rule
that intermediate commits carry a stale bundle and only the tip has to be
right.
dev'"'"'s CSS moved to index-Db2rfQf-.css while this branch was out; the
rebuild lands on the same hash, so static/index.html differs from dev by
the script line alone again.
The dev merge took dev's static/index.html, which loads the pre-restore
bundle, and left both bundles tracked. Merged as-is none of the frontend
shipped: no Restore button, no modal, no Type column.
Rebuild drops the superseded bundle and points index.html at a single one
that carries restoreFromGit and this round's new note leaf. The CSS
hashes identically to dev'"'"'s, so index.html differs by the script line
alone.
Two ways the K-profile and spool categories broke the
restored + skipped + failed == item_count invariant the settings count
holds:
* The spools preview counted only the spools and put the usage records
in the detail, but _restore_spool_usage increments the same tally, so
any backup with usage history reported a total larger than the number
the user was shown. The preview now counts both and the detail breaks
the total down instead of adding to it.
* A K-profile entry that is not a dict was dropped silently on the
connected path. _kprofile_profile_count includes it, so the offline,
printer-missing and step-failed paths all account for it; only the one
path that talks to a printer let it leave the tally. It now counts
failed.
The database phase had the same shape the K-profile phase did: _find_
archive, _find_spool, the usage dedupe and _restore_settings are all one
SELECT per row or per key, interleaved with autoflushed INSERTs, inside a
single open write transaction. A few thousand archives plus a full usage
history plausibly passes the 15 s busy_timeout, and every concurrent
writer in the app fails with "database is locked" until it finishes.
Each category now commits before the next starts. The id maps are plain
dicts in memory and the session is expire_on_commit=False, so the
ordering tolerates it.
The cost is that a later failure no longer rolls back an earlier
category, so a tally is recorded only after its category commits and
run_restore reports the categories already on disk instead of an empty
result - the same correction the K-profile split needed.
settings was gated on settings:update because a restore rewrites rows
PUT /api/v1/settings/ owns. The same argument applies to the other three
categories, and gating one but not the rest is the only state that is
not defensible: a role holding Backup alone could still write spools,
archives and K-profiles through a restore that it cannot write through
the endpoints that own them.
Each category now also requires that endpoint's write permission -
inventory:update, archives:update_all and kprofiles:update. archives
takes update_all rather than create because a restore writes rows owned
by other users, which is exactly what update_all means.
All missing permissions are reported in one refusal: a restore is a
multi-select, so naming them one at a time turns picking four categories
into four round trips.
The comment called the hint list belt-and-braces over keys the collector
already refuses to write. It is not: _collect_settings filters exactly
bambu_cloud_token and auth_secret_key, so a current backup really does
carry mqtt_password, ldap_bind_password, ha_token and prometheus_token,
and the hints are the only thing that refuses them. The companion-
credential rule sits downstream of that, so reading the list as redundant
and shortening it would write a stale credential and make that rule inert
at the same time.
Comment and test docstring only - no behaviour change.
created_by_id is only meaningful on the instance that wrote it. Restoring
onto a rebuilt instance - this feature's main use case - renumbers the
users table, so a live id can land on a different person and hand one
user's print history to another under archives:read_own. The id path
cannot even detect that: archivesOwnerCleared fires only for an id that
is absent, so a valid-but-wrong id produced no note at all.
The collector now records created_by_username alongside the id, and the
restore prefers it. username is unique on users, so a match is the same
person; the one case it cannot resolve - a user renamed since the backup
- falls through to ownerless with a note rather than guessing from the
id. The id stays as the fallback for backups taken before this change.
Configuring a slot from the printer card left the card showing the old
filament until a reload or the 30s fallback poll. The command reached the
printer and the printer applied it; the update just never got broadcast.
on_printer_status_change deduplicates WebSocket pushes against a status_key
whose AMS part carried id, tray_type and state. Configure Slot writes none of
those -- it writes tray_info_idx, tray_color, tray_sub_brands and cali_idx. So
PLA to another brand or colour of PLA produced an identical key and was
dropped, while PLA to PETG came through. Reset always worked because it clears
tray_type.
Those four fields only move when someone configures a slot or swaps a spool,
so this costs no broadcasts mid-print. remain stays out of the key for the
opposite reason.
Removing the requestAnimationFrame wrapper fixed the total stall but left the
100ms coalescing timer in the path, and a hidden page's timers are clamped to
once a second at best -- once a minute past five minutes hidden. The reporter
still saw a tab title at 2% beside a page at 40%.
The coalescing guards against a render cascade, which a hidden tab cannot
have, so it is skipped there and kept while visible.
The existing hidden-tab tests advanced fake timers, which simulates the timer
the browser was throttling; the new one never advances the clock.
Binds binary_sensor and reading-carrying sensor entities to a printer and
renders their state on its card, worded by Home Assistant's device_class.
Optional per-sensor alert condition drives a notification on the transition
into the alert state and an opt-in interlock that holds queued prints while
alerting -- a hold with a readable waiting_reason, never a failure, and only
ever on a sensor that was read successfully.
Sensors get their own table rather than a wider entity pattern on SmartPlug:
get_smart_plug_by_printer would otherwise hand the card's power button a door
contact to switch.
The hold is passed to the model matcher directly rather than merged into
busy_printers: _check_auto_drying reads that set as "is currently printing"
and would put an idle-but-held printer down the mid-print drying path.
The notification_providers migration spells its default FALSE, not 0 --
Postgres rejects an integer default for a boolean and _safe_execute swallows
the error.
Five X2Ds with no AMS, each printing from its external spool holder,
took a job sent to a named printer and refused the same job sent to
"Any X2D": the file uploaded, the firmware answered 0700_8012 "Failed
to get AMS mapping table", and the item failed after three attempts.
A named-printer job carries a mapping the frontend resolved at queue
time, so the scheduler's matcher never runs. A model-based job has no
printer until dispatch, so the matcher does run -- and could not see an
external spool on a dual-nozzle printer. _build_loaded_filaments derived
dual-nozzle status from ams_extruder_map, which is built from AMS info
bits, so a printer with zero AMS units reported an empty map; every
external spool got extruder_id=None, and the nozzle-aware hard filter in
_match_filaments_to_slots discarded it because None equals neither 0 nor
1. The mapping came back all -1, was cleared to None, and the print
command went out as use_ams:true with no ams_mapping and no
ams_mapping2 at all.
This is the backend half of #1257, which fixed the same logic in
useFilamentMapping.ts and left this copy behind. Mirror its inference:
a populated nozzles[1].nozzle_diameter, a non-empty ams_extruder_map, or
more than one vt_tray entry. Replaying the reporter's own push-status
now yields extruder 1 for Ext-L and 0 for Ext-R, and a nozzle-1
requirement resolves to [254] -- what their working named-printer
dispatch sent. Single-nozzle printers keep extruder_id=None; nozzles
always has two entries, so its length alone must not be the signal.
Also stop dispatching a job the firmware is certain to reject. When the
matcher ran, matched nothing, and the printer has no AMS, fail the item
with the filament and nozzle it wants instead of spending an upload and
two retries on it -- that path already ended in a failed item, just an
opaque one. With an AMS attached the firmware error still stands, since
there the user can load a spool and press Resume. Fail-safe like the
nozzle-diameter guard (#1899): every branch short of a positive finding
returns None and dispatches as before.
_apply_filament_overrides is extracted from _compute_ams_mapping_for_printer
so the message names the filament the matcher looked for rather than the one
the 3MF was sliced with.
An H2D started a twelve-hour PETG dry at 65 degC and the AMS gave up on it
twenty minutes in, with 700 of the 720 minutes still on the clock. It cooled,
humidity climbed back over the threshold, auto-drying started another
twelve-hour cycle, and that one went the same way; the reporter's AMS
temperature history shows the loop running all morning.
The log had one line for it: "AMS 0 drying complete", which is exactly what it
says for a dry that ran its full twelve hours. Nothing in a support bundle told
the two apart, and the one number that does -- the time still remaining -- was
written into that line as the previous value, where it reads like a duration
rather than a shortfall. The reporter took 700 for seconds and concluded the
cycle had lasted twelve minutes.
Bambuddy did not stop that cycle; every stop it sends is logged with the full
outgoing command and there was none. So ending it was the printer's decision,
and the account of why lives in three things already received and parsed and
never written down: the drying phase and sub-phase from the AMS info hex, the
per-unit dry_sf_reason constraint codes, and the live HMS errors.
A cycle that ends with most of its countdown left now logs all three alongside
how much of the requested duration ran. One that reaches its duration keeps the
single line it has always had. A stop Bambuddy sent is named as ours -- it is
short of its duration too, and on the telemetry alone is indistinguishable from
the firmware abandoning the cycle, so without tracking it the print-takes-priority
stop and the Stop button would both have been blamed on the printer.
Diagnostics only. Nothing about when drying starts or stops has changed, and the
restart loop is not addressed: what the firmware objects to has to be established
before Bambuddy can sensibly decide how long to wait before trying again.
The desktop handoff accepted library:read alongside read_all/read_own, on
the reasoning that default groups do not carry it and requiring it would
lock out Operators and Viewers. The permission grants nothing in that
position: the slicer-token endpoint gates on
require_ownership_permission(LIBRARY_READ_ALL, LIBRARY_READ_OWN), and
neither that dependency nor User.has_permission expands the legacy name,
so a group holding only library:read gets a 403 there. It cannot reach
the File Manager to try, either - GET /library/folders gates on the same
pair - and the library:read -> library:read_own migration in
core/database.py runs only over the groups named in DEFAULT_GROUPS, so a
custom role that still carries it stays stuck rather than being upgraded.
custom role that still carries it stays stuck rather than being upgraded.
Accepting it only enabled a menu item the server refuses, and the failure
is indistinguishable from "no slicer installed" once the catch hands the
unauthenticated URL over. Removed, with the comment recording the reason
so the next reader does not re-add it, and a test that pins it.
---
refactor(slicer): share one sliceable-file-type rule (#2725)
The File Manager and the 3D preview decide the same thing about the same
file and each held its own list of extensions - which is how they came to
disagree, offering a desktop handoff for an STL whose own preview showed
"Open in Slicer" greyed out. Making the two lists identical fixed the
symptom and left the drift, so SLICEABLE_FILE_TYPES now lives in
utils/slicer.ts with isSliceableFileType for a stored file_type and
isSliceableFilename for a name.
The filename form still rules out the compound extensions explicitly,
since .gcode.3mf ends with .3mf; the type form does not need to, because
classify_file_type stores that one whole.
Both test files mocked the whole slicer module, which would have replaced
the new predicates with undefined - switched to importOriginal so only
openInSlicer is stubbed. That is the better shape regardless: the tests
now exercise the rule the component runs instead of a copy declared
beside them.
Carries the rebuilt bundle. The CSS hash moves with it - the split button
introduces Tailwind classes the previous build had no reason to emit.
The three tests around it use flat scalars, which is also all the field's
placeholder and the wiki showed, so nothing recorded that the value is
forwarded verbatim rather than treated as a key/value list. A reporter asked
whether action buttons work; they always have, and now that is pinned.
The changelog entry said "nested options work" and left it there. It now names
actions and the two things that decide whether the buttons do anything - the
mobile_app_notification_action automation, and iOS needing a registered
category - since neither is set from Bambuddy and both are what a reader would
otherwise have to discover the way the reporter did.
Every LDAP user on an lldap directory was rejected with "Incorrect
username or password", on an install where Test Connection passed and
where the same bind DN, filter and group membership all checked out
under ldapsearch. The directory never saw the request.
_extract_user_info searches for POSIX groups alongside the memberOf
ones, and both of those filters name the posixGroup object class. ldap3
fetches the schema at connect time (get_info=ALL) and validates class
names in a filter against it while building the request, raising
LDAPObjectClassError before anything is sent. lldap marks every account
it creates as posixAccount -- which is what makes us look for POSIX
groups at all -- but defines no group class beyond groupOfNames. The
exception escaped authenticate_ldap_user, and the login route reports
any LDAP failure as bad credentials.
A directory with no posixGroup class has no posixGroup entries, which is
exactly the answer those searches would have returned. Catch it, log it
once, and carry on with the memberOf groups collected above. Both
searches sit inside the one try: they name the same class, so once one
is rejected the other cannot succeed, and attempting it would only
produce a second identical exception to swallow.
Not a regression from 848f55810. The memberUid filter has named the
class since b6599dd41 and runs for every user whether or not they have a
gidNumber, so a directory of this shape has never been able to log in;
the primary-group lookup only added a second trigger. Test Connection
was unaffected throughout because (objectClass=*) is a presence filter
and never reaches the value validator.
The mock connection gained a hook that raises on a filter substring,
standing in for that client-side validation. Reset in the fixture and
default None, so existing tests are unchanged.
A sliced file numbers its filaments 1..4; which AMS tray each came from is
a separate decision made when the job is sent. store_print_data learns it
from one of two sources, both of which require the print command to pass
through us: the mapping Bambuddy chose itself, or the one it intercepted
on the printer's local request topic. A job dispatched from Bambu Studio
while the printer is cloud-bound satisfies neither -- the command travels
through Bambu's broker and never reaches the topic we subscribe to.
slot_to_tray is then NULL and _resolve_global_tray_id guesses by position:
filament 1 from the first loaded tray, filament 2 from the second. The
reporter's X1C was loaded in the order 2, 4, 1, AMS-HT, so all four slots
were charged to the wrong spool. Their log carries the printer's own
answer, mapping=[1, 3, 0, 32768], sitting unread.
usage_tracker has consulted that field since it started resolving mappings
at completion, along with a colour match against the loaded trays for the
models that never publish it (A1, A1 Mini, P1S, P2S). Only the Spoolman
writer, which resolves at print start, never learned to -- and main.py
gates usage_tracker behind Spoolman being off, so enabling Spoolman is
what costs you the better resolver.
_resolve_slot_to_tray_fallback gives it both, at completion rather than at
print start: a printer keeps publishing the last job's mapping while it
sits idle, so reading it early would risk stamping the previous print's
mapping onto this one. A mapping we or the slicer actually recorded is
never second-guessed.
Applied in _report_partial_usage too. Cancelled and failed prints feed the
same slot_to_tray to the same resolver and mis-charged just as readily.
The resolved mapping and its source are now logged at print start and at
completion. "source: none" at start is the signal that completion will
have to fall back, and it was the one line that would have turned this
report into a five-minute triage.
Not addressed: editing the mapping after the fact, which the reporter also
asked for. ArchiveUpdate exposes neither filament field and there is no way
to re-run an attribution, so that is a feature rather than a fix.
The follow-up to the same report: a second AMS 2 Pro, no aux power,
loaded entirely with PLA and drying at the 45C the reporter picked,
showed 45C and then switched to 55C.
Bambu never echoes back a cycle's filament or temperature, so both come
from the target cached when the command went out, and the fallback for
a missing cache reads the loaded trays. The first pass narrowed that
fallback to units whose spools agree on a filament, which fixed the
mixed-unit case in the original report but left the uniform case
answering with the spools' RFID-recommended drying_temp -- 55C here.
Agreement across slots is evidence of what is being dried, because the
dryer heats all of them. It is no evidence of the temperature, which is
picked freely in the popover, so the recommendation was never more than
a guess wearing the same confident "PLA @ 55C" as a known target.
uniform_tray_drying_hint therefore becomes uniform_tray_filament_hint
and returns the filament alone. The badge names a temperature only when
we sent it, and otherwise shows the filament and the countdown.
Both status builders also stopped filling the two fields independently.
Entering the fallback when either was missing let a cached filament pair
with a guessed temperature and render as though both were known; the
temperature now simply has no fallback to reach.
The badge required both fields before rendering anything, so dropping
the temperature would have blanked it rather than shortening it -- the
frontend now renders each on its own terms. No new translation key: the
filament type is a passthrough.
This changes what is shown when the cached target is missing, not why
it goes missing. If the reporter was on the fixed build, the falling-
edge gate is still letting a zero through on an unpowered unit, which
needs a log covering the start of the cycle.
5.0.8's maxLength cap was applied in combine(), where output is merged, but
not to the two arrays built before it runs: comma alternatives each got their
own full allowance and were concatenated with no running total, and padded
sequences never consulted maxLength at all. So a ~25 KB pattern still OOMs the
process -- fatally, past the reach of try/catch -- and a ~400 KB one blocks the
event loop for over two minutes. 5.0.9 bounds both as they are built.
Dev-only and transitive here: it reaches us as eslint -> minimatch@5 ->
brace-expansion, the only input it sees is our own lint globs, and it is not in
the shipped bundle. The ci.yml audit gate runs --omit=dev, so this never would
have failed CI; it surfaced through Dependabot.
The overrides floor is bumped alongside the lockfile so a clean install can't
resolve back to the vulnerable 5.0.8.
A restore writes a `github_backup_logs` row too — same table, same status
values, and it already carried `trigger: 'restore'`, which the API already
returned. The history table rendered date / status / commit only, so the row
read as a successful backup dated now while "Last backup" said something
else: `last_backup_at` is only stamped by an actual backup, and the two
disagreeing is alarming with nothing on screen to explain it.
Adds the Type column the trigger was always there to fill. Unknown values
fall back to the raw string rather than rendering blank, matching the
`backup.pathCheck.*` lookup a few hundred lines up — a trigger kind added
later shows up as itself instead of vanishing.
Backend unchanged: it has recorded this correctly since the restore path was
written.
Three tests, all three failing without the column. 13 locales in parity at
5776 leaves — pt-BR takes "Backup manual" rather than the parenthesised form
because "Backup (manual)" is identical to en, which the parity check counts
as untranslated.
Bundle rebuilt: `index-DhOfNgMz.js` → `index-CadgB7UN.js`. It also picks up
the `archivesOwnerUnknown` leaves from the previous commit, which changed
i18n without rebuilding.
`set_kprofiles_batch` defaults the field with
`p.get("nozzle_id", f"HS00-{diameter}")`, and a `dict.get` default only
fires when the key is *absent*. The restore always set the key —
`"nozzle_id": p.get("nozzle_id")` — so a backup that carries no nozzle_id
published `nozzle_id: null` to the printer instead. Printers that omit the
field (KProfilesView's own #1748 comments) are exactly why the default is
there, and it was unreachable from this path.
Set only when known, on the same precedence the `setting_id` line beside it
already uses: the live profile first, then the backup, then absent. Live
first is the point rather than a bonus — `nozzle_id` encodes the fitted
nozzle's type as well as its diameter (`HS00-` hardened vs `SS00-`
stainless), so a nozzle swapped since the backup makes the stored value
stale, and the write lands on the nozzle fitted now.
Read with `getattr`, matching the defensive read of `extruder_id` in
`_match_kprofile`: not every live profile carries every field, and
`test_a_live_index_that_reports_no_extruder_still_matches` is the standing
control for that.
Four tests, two of which fail without the fix. The `_live` double also
gained `nozzle_id` — it is a non-default field on the real `KProfile`
dataclass, so omitting it let the double license code the real object
would have accepted.
`created_by_id` is not attribution, it is the column the access check runs
on: `_ensure_archive_visible` fails closed on NULL, so an ownerless archive
is a 404 for every caller without `archives:read_all` and never appears in
the ownership-scoped list queries.
On the overwrite path an absent key correctly leaves the local owner alone
— that rule is deliberate and unchanged. On the insert path there is no
local row to fall back on, so the archive lands ownerless, and nothing said
so. The restore reported N archives restored while the user who asked for
them saw none. Two ways in, both silent: a commit taken before the
collector recorded the column (every pre-#2656 backup), and an archive that
genuinely had no owner on the source instance.
Adds `archivesOwnerUnknown`, emitted on insert only, and suppressed when
the stale-id branch has already spoken for that row so one cause does not
produce two notes. Wording mirrors `archivesOwnerCleared` because the
consequence and the remedy are the same; the cause is not, so it is a
separate code rather than a reuse.
Five tests, plus the existing `test_a_backup_without_the_key_still_restores`
renamed and tightened — it asserted the silence this fixes. 13 locales back
in parity at 5772 leaves. No modal change: notes render through
`translateCoded`, which resolves by code.
Both `created_at` columns the restore dedupes on are
`server_default=func.now()`. SQLite fills those from `CURRENT_TIMESTAMP`,
which has second precision and stores `'2026-08-02 11:28:41'`, while
SQLAlchemy binds a Python datetime as `'2026-08-02 11:28:41.000000'`.
SQLite compares the two as strings, so `Model.created_at == created_at`
never matched a row the application itself created — not even when handed
that row's own value straight back out of the ORM.
Every dedupe keyed on it therefore missed, on the ordinary case rather
than an edge one:
* `_find_spool`'s composite fallback duplicated every tag-less spool on
each restore, and `overwrite_existing=True` never reached the original;
* the usage-history dedupe re-inserted the user's entire consumption
history on each restore.
Rows the restore itself had inserted did match, because those carry an
explicit bind in the same microsecond format — which is why the existing
repeat-restore tests passed throughout.
Fixed by filtering the candidates in SQL and comparing `created_at` in
Python, which sidesteps the bind format and behaves identically on
PostgreSQL, where the column keeps microseconds and the SQL comparison
happened to work. `_parse_dt` now also normalises an offset-bearing value
to naive UTC, matching what the naive columns actually hold; the collector
never writes one, so that guards hand-edited and foreign backups.
Seven tests, six of which fail without the fix. They seed the "existing"
row the way the application does — no explicit `created_at` — which is
what the existing coverage was missing.
`_apply` commits the database categories before the K-profile phase, and the
comment there is right about why: `get_kprofiles` is 3 x 5 s per printer per
nozzle and SQLite's `busy_timeout` is 15 s, so holding the writer across the
MQTT phase would fail every concurrent writer in the app.
But `run_restore`'s handler returns `{"success": False, ..., "results": {}}`
for anything raised after that point, and the per-call guards inside
`_restore_kprofiles` do not cover the whole phase. Two consequences, and the
second is worse:
* The user is told the restore failed and handed an empty `results` while the
archive, spool and settings rows are durable on disk. The honest-reporting
theme this whole feature is built on inverted on exactly the path where it
matters most.
* `_reconfigure_mqtt_relay` sits inside the same `try`, downstream of the
raise. A restore that rewrote the mqtt_* rows left the relay pointed at the
pre-restore broker until something else reconfigured it.
`_apply` now contains the K-profile phase: fold the error into that category's
tally as `failed` plus a `kprofilesStepFailed` note, and let the results it has
already committed be returned and reported. Every profile the payload carried
and the phase did not account for is counted failed — silence would have been
the same lie in a smaller font. `_reconfigure_mqtt_relay` is reached again
because `_apply` returns normally. The rollback in the handler discards only
the phase's own read transaction, so a database error cannot leave the session
in a state that turns the caller's commit into the very report this prevents.
`kprofilesSendFailed` was the obvious note to reuse and is the wrong one: it
names a nozzle, a printer and a serial that a phase-level failure does not
have, and "failed to send" is untrue of a step that never got as far as
sending. One new leaf x 13 locales instead.
Belt-and-braces on the trigger that found this:
`sum(len(c.get("profiles") or []) ...)` raises TypeError on a hand-edited or
truncated backup whose `profiles` is not a list, and it runs before the guards.
Counting defensively makes that a skipped category rather than an exception
thrown over committed rows.
Control kept explicit: a failure *before* the commit still rolls back, still
reports nothing restored, and still does not touch the relay.
Tests: +5 (280 -> 285 across the three restore files, 328 -> 337 across
`-k github`). Fail-pre-fix 4 — 3 for the containment, 1 for the defensive
count, checked separately. i18n parity 13 locales at 5771 leaves.
Bundle rebuilt for the new leaf: index-CHCEEMgx.js -> index-DhOfNgMz.js. CSS
hash unchanged.
`if not isinstance(total, int) or seen >= total or not entries: return blobs, ""`
— the first arm short-circuited the page loop into a **success** holding page 1
only. Gitea clamps `per_page` to `MAX_RESPONSE_ITEMS` (default 50), so that is
50 entries of an arbitrarily large tree returned as a complete listing.
The restore then reports genuinely-present categories as "Not present in this
backup commit". That silent skip is the exact failure this override exists to
prevent, and the same class as E7 and G2 — G2 fixed the arithmetic here and
left the shape. GitHub and GitLab both hard-fail in the equivalent spot; only
Gitea guessed, and it guessed in the one direction that loses data quietly.
Whether Gitea always sends `total_count` on this route is beside the point: the
code was defending against a response shape it did not trust, and then trusting
it.
Now a missing or non-int `total_count` means "page until a short or empty
page". A page shorter than the first one is the last one, floored at Gitea's
default clamp so a genuinely small tree still costs exactly one request — the
reason G2 rejected paging-until-short in the `total_count`-present case, which
is unchanged and still stops on the count. The existing `page <= 50` ceiling
gives the correct hard failure for a tree that really is over cap, so this
cannot truncate.
Residual, and deliberately not widened into a `return None` on the first
ambiguous response — that would break single-page trees, the common case: an
instance whose `MAX_RESPONSE_ITEMS` is set *below* 50 *and* which omits
`total_count` would still stop at page 1. Both halves have to be true.
Tests: +6 (paged to the end with no count, on both Gitea and Forgejo; a short
page ends it; a non-int count is treated as no count; the page ceiling still
fails). Fail-pre-fix 5, control that passes either way 1 (a small tree is one
request). These don't match `-k github`, so 274 -> 280 across the three restore
files but `-k github` is unmoved.
`_match_kprofile` scoped candidates by `filament_id` alone, and
`_current_kprofile_index` reads the live index per nozzle *diameter* — so on a
dual-nozzle printer both extruders' profiles come back in one list.
On an H2D with the same filament calibrated on both extruders, the `setting_id`
arm then matched whichever profile the printer happened to list first. A
backed-up extruder-0 entry took extruder 1's slot and went into the batch as
`{extruder_id: 0, cali_idx: <extruder-1 slot>}`, writing one extruder's
calibration over the other's and counting it restored. With an entry per
extruder — the ordinary case, since the same preset on both nozzles is what a
dual-nozzle printer is for — the two swapped slots and clobbered each other.
The `name` arm and the single-candidate fallback were equally unscoped, so this
was never only about the ambiguous case.
The data was already in hand and already being read: the backup entry carries
`extruder_id` (`:1546` copies it straight into the outgoing dict) and
`KProfile` has carried `extruder_id: int` on the live side all along. Scope
`candidates` by it the same way `filament_id` already scopes them, and the
single-candidate fallback narrows with them — ambiguity is judged within one
extruder now.
Conditional on both sides saying which extruder they mean. A pre-#2656 backup
has no `extruder_id`, and a live index that reports none must not turn every
entry into an add — that would be a far worse regression than the bug. Two
controls cover each direction of that.
`claimed` (G3) is untouched: it is per nozzle-loop and this only narrows the
candidate set feeding it.
Tests: +4 across the three restore files (270 -> 274). Fail-pre-fix 2 (each
extruder keeps its own slot; the other extruder's profile is not a stand-in),
controls that pass either way 2.
This modal carried two workarounds for #2716: `onSuccess` deliberately did not
invalidate `['settings']`, and a query-cache subscription pinned the entry to
the pre-restore copy for as long as the result panel was up. Both existed
because SettingsPage's debounced auto-save diffed its `localSettings` form
state against the live cache, so any refetch of a restored settings row --
this modal's, a window refocus, a reconnect, or any of the ~30 other observers
of the key -- read as an edit and PATCHed the pre-restore values back over the
restore about 500 ms later.
`43cb216a` on dev fixed that. The page now keeps a server baseline and
reconciles a moved snapshot field by field: an untouched field adopts the
server's value instead of overwriting it. The restore no longer needs an
exception, and maziggy explicitly invited dropping it.
A commit on top rather than a rebase-drop of `21bb5afc`: later commits touch
this file, and the workaround was right when it was written. This says so.
The reload on close stays -- it was never one of the two workarounds. Its
stated reason was, though, and it was the #2716 bug, so it is restated for
what it actually buys: invalidating `['settings']` only resyncs what reads
that query, and the interface language, currency and auth toggles are read on
boot.
Tests: "never invalidates the settings query" inverts; the pin test and its
control go with the pin. The reload pair stays. 28 -> 26 tests in this file.
`18938a10` on `dev` changed `set_kprofiles_batch` from returning a `bool` to
returning the sequence_id it published the command under, and moved the
verdict to a separate `await client.await_cali_ack(seq)` returning
`(ok, detail)`. Every caller in `api/routes/kprofiles.py` was updated with it.
`_restore_kprofiles` was not — it still did `sent = client.set_kprofiles_batch(...)`
and branched on `if sent:`.
A sequence_id string is truthy, so that compiled, passed, and silently made
the restore the one path left in the codebase that reports a refused
K-profile write as saved — exactly the defect `18938a10` closed everywhere
else.
Keep the sequence_id, await the ack per batch, and route an explicit refusal
into `tally.failed` with a new `kprofilesRefused` note carrying the printer's
own `reason`. Reusing `kprofilesSendFailed` would have been wrong: the
command was sent, and the printer answered.
Silence still counts restored. That is `await_cali_ack`'s own contract and
the maintainer's rule — no answer is not evidence of refusal, and firmware
predating the ack never answers. An exception reading the ack degrades the
same way rather than inventing a failure out of a write that most likely
landed.
`kprofilesAckUnreliable` is reworded to match: a refusal is now believed, so
the caveat narrows to what is genuinely left uncertain. The ack is only worth
reading at all because `18938a10` also changed the payload's `tray_id` from
`-1` to `0` — single-nozzle firmware answered `result: "fail"` to `-1` on
writes that demonstrably applied. This restore builds no `tray_id` of its
own, so it inherits that fix for free.
Tests: 4 regression (the ack is awaited for the returned sequence_id; a
refused batch counts failed and surfaces the printer's reason; one refused
nozzle does not condemn the other; the reworded caveat) + 3 controls (a
silent printer still counts restored; an unreadable ack does not fail the
batch; `None` keeps the existing send-failed path and awaits nothing). All
four confirmed failing against the pre-fix service.
With overwrite off, the confirmation said "Missing entries are added; existing
entries stay as they are." For archives, spools and settings that is true --
_apply threads the flag into all three. _restore_kprofiles takes no overwrite
parameter at all, and deliberately: writing a slot is always an overwrite on the
printer, so it resolves the live cali_idx and publishes extrusion_cali_set
either way, replacing whatever calibration that slot currently holds.
The behaviour is right and the backend does say so, but it says so as a
kprofilesAlwaysOverwrite note -- which only reaches the user in the result panel,
after an MQTT send that cannot be taken back. The one screen that explains
overwrite-off stated the opposite. So the fix is on the frontend, where the
mismatch is.
One new leaf, kprofilesOverwriteCaveat, in all 13 locales, rendered in two
places: appended to the overwrite-off confirmation when kprofiles is among the
selected categories, and beside the K-profiles row itself as soon as it is
ticked, which is the same screen as the toggle whose promise it qualifies.
Neither appears with overwrite on, where nothing is promising otherwise.
Tests: 2 that fail pre-fix (the caveat beside the row, and inside the
confirmation the user clicks through) and 2 controls (a spools-only restore keeps
the plain message; overwrite-on keeps the strong one and adds nothing). Frontend
suite 2601 -> 2605 tests across 195 files.
The companion-credential rule has five conditions, and the second one -- "the
backup itself carried a usable credential" -- was applied to all five pairs. It
should not be. It is what stops the rule over-refusing an anonymous MQTT broker
or an anonymous LDAP bind, both of which are working configs: there, an empty
credential in the backup means the restore is not producing anything weaker
than what was backed up.
For prometheus_enabled it does not transfer. An empty prometheus_token removes
/api/v1/metrics' only gate, so the exposure is a property of the toggle, not of
a downgrade relative to the backup -- and prometheus_token is optional, so a
backup taken on an instance that enabled Prometheus without ever setting one
carries the toggle and no usable token. That payload skipped the refusal
entirely: not a candidate, so the local-state pass never ran, and the blocklist
quietly dropped the token key. On a token-less target the result was
prometheus_enabled=true, no token row anywhere, and a full unauthenticated
metrics dump -- the same hole the rule was written to close, reached from the
likelier of the two directions.
So condition 2 is now per-pair: an exposure class (prometheus) that skips it and
is judged on local state alone, and an availability class (mqtt, ldap, ha,
virtual_printer) that keeps it. Nothing else changes -- the local-state pass
already stands down when the instance has its own credential, when HA_TOKEN is
in the environment, and when the toggle is already on locally, so "the exposure
pre-dates this restore" still holds and refusals still get no tally increment.
One wording consequence: an exposure toggle can now be refused on a payload with
no credential-like key in it at all, where the shared caveat would have read "0
credential-like key(s) will be skipped". That case gets its own preview detail
code, settingsCompanionOnlyWillSkip, in all 13 locales.
Tests: 6 that fail pre-fix -- the token key absent and blank at the unit level,
the new preview wording, and the integration test through the real endpoint for
both payloads (200 with a full metrics body before this, 404 after). Plus 3
controls, because over-refusal is still the real risk: the exposure route must
still stand down for a local token and for an already-on toggle, and the
availability class must still let a credential-less mqtt/ldap/virtual_printer
toggle through. The anonymous-broker and anonymous-bind controls are unchanged
and still pass.
Both docstrings said the collector never writes `cloud_profiles/*.json`. That
was true when they were written and stopped being true at `455a9e4b` (#2717,
"collect cloud profiles from every connected account"), which is this
branch's rebase base — so the PR was shipping a stated reason its own base
had invalidated.
The real reason is the one the PR body now gives: restoring a preset means
writing to a Bambu or Orca Cloud account, which is a different operation from
every other category here. Those land in the local database, or on a printer
the instance already owns.
Checked that nothing in the restore path is confused by the new files — the
category globs don't reach `cloud_profiles/`, and it stays out of
`RestoreCategory`.
The restore endpoint was gated on `github:restore` alone, and the settings
category rewrites arbitrary non-auth `Settings` rows. Backup and Settings are
separate permission groups, so a role holding only Backup could change
settings it cannot change through `PUT /api/v1/settings/`, the endpoint that
owns them.
The inconsistency is ours rather than an inference: this module already makes
exactly this argument — it is why the four protected auth keys are refused
outright — and `library.py` sets the precedent of elevating a route to
`settings:update` for the same reason.
Gated per-category rather than by demoting `github:restore` wholesale, so it
stays narrow and doesn't presume the answer for `spools:*`, `archives:*` and
`kprofiles`. That broader permission-model question goes to the maintainer in
the PR reply.
`current_user is None` only means auth is disabled — `github:restore` is in
`_APIKEY_DENIED_PERMISSIONS`, so an API key never reaches the route body.
No frontend change: `request()` puts the 403's `detail` on the Error, and the
modal already renders `restoreMutation.error.message` in its red block, so
the user sees the missing permission named.
Tests: 1 regression (a Backup-only role gets 403 and `run_restore` is never
awaited) + 3 controls (the same role still restores the other three
categories; a role holding both permissions still restores settings; auth
disabled is unaffected). The regression confirmed failing against the pre-fix
route. `_create_config` gained an optional token so it works under auth.
`_match_kprofile` ends in a single-candidate fallback, and the per-nozzle
loop called it once per entry with no record of which live profiles were
already taken. Two backup entries sharing a `filament_id` and matching on
neither `setting_id` nor `name` both resolved to the same live profile, both
got the same `cali_idx`, and both went into the batch — so the second
overwrote the first on the printer while the tally counted two restored.
Reachable in the ordinary way: the user deletes one of a pair after the
backup, and the delete-then-add re-key this code already reasons about is
exactly what strips the `setting_id` match.
Fix: thread a `claimed` set of slot ids through the loop; a live profile can
only stand in for one entry. A displaced entry falls through to
`cali_idx: -1` — add-as-new is the safe outcome — and folds into the
existing `kprofilesUnmatched` note rather than earning a new code.
The single-candidate fallback is still judged against every candidate rather
than the unclaimed ones. Two live profiles for one filament are ambiguous
whether or not another entry has taken one, and narrowing to "available"
would turn a guess the code deliberately refuses into a match.
Tests: 2 regression (the displaced entry is added rather than aliased, and
keeps its own setting_id) + 2 controls (two genuine matches keep their own
slots; a claimed slot does not make an ambiguous pair matchable). Both
regressions confirmed failing against the pre-fix service.
The pager computed `seen = (page - 1) * 1000 + len(entries)`, taking the
requested `per_page` as fact. Gitea clamps `per_page` to
`MAX_RESPONSE_ITEMS`, which defaults to 50. So on a default install page 1
returns 50 entries and sets `seen` to 50, then page 2 sets it to 1050 —
which clears any `total_count` under 1050. The loop returns `success: true`
holding the first 100 entries of a much larger tree.
The restore then reads every missing path as "category not present in this
commit" and skips it silently, which is precisely the failure this override
was written to prevent. Same class as the GitLab pager fix, in the one
direction that got left behind.
Fix: accumulate `seen += len(entries)`. A genuinely over-cap tree still
hard-fails rather than truncating; the cap is a page count, not a file
count, because the page size is the server's choice.
Test: a 120-entry tree served 50 at a time reaches its last entry, in three
requests. Confirmed failing against the pre-fix backend — it stopped after
two pages and reported 100 entries as the whole tree.
`created_by_id` and `deleted_at` both went into the archive `fields` dict
unconditionally, via `entry.get(...)`. A backup commit taken before the
collector wrote those keys carries neither, so `.get` yielded None for both
and the overwrite branch — a blanket `setattr` over every key — wrote NULL
over a live owner.
That is exactly the failure carrying `created_by_id` was added to fix, only
now inflicted on rows that were fine: `_ensure_archive_visible` fails closed
on a NULL owner, so the archive 404s for the person who owns it. It emitted
no note either, because `archivesOwnerCleared` only fires for an id that
isn't in `valid_users`, not for an absent key — and the row still counted as
restored. `deleted_at` had the mirror problem: an old commit silently
un-deleted, since `archivesUndeleted` reads the same absent value.
Absent is not the same as explicitly null. Both keys now only enter `fields`
when the entry actually carries them, so an old commit leaves the column
alone on overwrite and a current one can still say "this archive has no
owner" or "this archive is live". Same shape as the tag-column rule: don't
clear what the backup doesn't know about.
Behaviour change to an existing test, called out deliberately:
`test_overwrite_undeletes_a_locally_deleted_archive_and_says_so` now has to
put `deleted_at: None` in the entry to mean it.
Tests: 2 regression (owner and deleted_at both left alone by a key-less
entry) + 2 controls (an explicit null is still honoured, with its note).
Both regressions confirmed failing against the pre-fix service.
The three remaining review items, all in the read path.
E1 — four provider calls where two would do. preview() called list_commits
twice: once inside _resolve_ref to turn HEAD into a SHA, once more at limit=20
purely to find the entry describing that same SHA. And list_tree's recursive
tree GET was thrown away, so fetch_files immediately fetched the identical tree
again to map path -> blob SHA. _resolve_ref now returns the entry it already
has, and list_tree returns its blob_shas map for fetch_files to take as an
optional argument. GitLab reads files by path and ignores it.
E2 — `commit: null` for a ref outside the 20 most recent. Two causes, and the
second is the one that actually bit: REF_PATTERN accepts a 7-character ref while
providers return the full 40, so the exact `==` in the scan never matched an
abbreviated SHA *even when the commit was in the window*. Fixed by prefix
comparison, plus a get_commit(ref) on the GitHub and GitLab backends for the
genuinely-outside-the-window case. Gitea and Forgejo inherit GitHub's. Still
best-effort: it is a subject line and a date, so a failed lookup renders the
preview without them rather than failing it.
E7 — the two tree readers disagreed, and each was wrong in the other's
direction. GitHub's recursive trees endpoint is not paginated and signals
overflow with truncated=true, which _blob_shas_at hard-fails on. Gitea and
Forgejo *do* page that endpoint, and inherited that single GET unchanged — so a
large backup repo returned only the first page and every category beyond it
looked absent from the commit. GiteaBackend now has its own paging
_blob_shas_at. GitLab had the mirror-image bug the review did not name: at its
50-page cap it exited through the while condition and returned success: True
with a silently partial path list. Both now fail loudly, which is what the
GitHub version was always doing.
Both halves of E7 are the same failure the module already refuses to allow: a
restore that skips categories and calls it "not present in this backup commit".
24 new or changed tests, all failing against this commit's parent.
Four of the review's smaller items.
E6, the substantive one. tag_uid and tray_uuid are both in the overwrite setattr
loop, so a spool matched on one key got the backup's *other* key written onto it.
Neither column has a unique constraint (models/spool.py, and no unique index in
the migrations), so nothing errors — a duplicate tag simply appears, after which
_find_spool's .scalars().first() is non-deterministic and an AMS tag lookup
resolves to an arbitrary one of the two spools. The same loop could also clear a
tag the user had scanned since the backup was taken, when the backup entry held
None.
_find_spool now reports which key matched, and _guard_tag_overwrite drops a tag
column from the write when the incoming value is empty and the local row has one
(the backup predates the scan, so the local tag is the newer fact) or when
another local spool already holds it. Announced in the tally the way the archive
un-delete case already announces itself, rather than done silently — the
spoolTagKept locale key landed with the rest of the i18n block last commit.
E5. The Restore button is hidden without github:restore. All three endpoints are
gated on it server-side, so the modal 403s on its first preview; offering the
button is offering an action that cannot work. Button only — the card stays
visible, since configuring backups is a separate permission — and hasPermission
returns true with auth off, so a single-user instance is unaffected.
E3. models/github_backup.py: the trigger comment said manual/scheduled; this PR
added a third value.
E4. ha_token_from_env: recommending no change, with the reasoning recorded as a
test rather than left in a review thread. It is built only in the settings GET
response, is absent from AppSettingsUpdate, and is therefore never a Settings
row — it cannot reach a backup, so an allowlist entry would be dead code. Worse,
a name-shaped exception to a belt-and-braces denylist is a live hole: an
attacker-authored settings/app_settings.json could get a *token*-named row
written by choosing that name.
4 unit tests and 1 frontend test that fail against this commit's parent, plus 4
controls: a free tag is still written, an unchanged tag is not reported as kept,
an insert is unaffected, and the button still shows with auth disabled.
A German user got a translated modal with "Not present in this backup commit" in
the middle of it. Every tally note and preview caveat was a server-built English
sentence rendered verbatim.
Follows the backup.pathCheck contract already in use one card down in the same
component, deliberately rather than inventing a second convention: the server
sends a `code` plus typed `params` and carries the English along as the
fallback, and the client renders
`t(`...${code}`, { ...params, defaultValue: message })`. The defaultValue arm is
what keeps a newer backend's unfamiliar code readable instead of printing the
raw key — covered by its own test.
Shapes:
- notes: list[str] -> list[GitHubRestoreNote] {code, params, message}. Breaking,
but the field is unreleased in this same PR.
- GitHubRestorePreviewCategory gains detail_code / detail_params; `detail` stays
as the English fallback.
- _CategoryTally.note(code, message, **params), deduped on (code, params) rather
than on the rendered text, so two offline printers both keep their names. The
20-note cap is unchanged.
28 new leaves across 13 locales: 20 notes.* and 8 details.*. `noData` collapses
the four per-category "No X data in this backup" strings into one, since the
category heading already renders beside it. Counts use single-form {{count}} in
the existing "N record(s)" style rather than i18next plural suffixes — nothing in
this block uses _one/_other and the parity script has extra rules for them.
Parity holds at 5737 leaves in all 13 locales.
Deliberately out of scope, and worth saying so rather than leaving it to look
like an oversight: result.message, the commit-picker subject lines and the HTTP
error strings stay English. Those also originate in the provider backends, so
code-ifying them widens the diff well past the restore service.
spoolTagKept is added here with the rest of the locale churn but is not emitted
until the next commit, so the 13-locale change lands once.
Two buttons labelled "Restore" were visible in the same viewport — this card's
and the Local Backup card's — with different destinations. The modal title
disambiguated them; the buttons did not.
backup.restoreFromGit.button only. The Local Backup card's t('backup.restore') is
unchanged, and each locale's new value follows the wording that locale already
uses in the modal title rather than being a literal translation of the English.
One correction to the review's aside: this does not simplify
GitHubRestoreModal.test.tsx. The /Restore$/ + confirmButtons[length - 1] idiom
there is not caused by the card button — the modal is rendered standalone in
those tests — but by the modal's own footer action and its confirm dialog both
reading t('backup.restore'). Left alone as out of scope.
Neither _collect_archives nor _restore_archives touched created_by_id, so every
restored archive row landed NULL. That column is not attribution, it is what the
access check runs on: _ensure_archive_visible (api/routes/archives.py) fails
closed on NULL — a 404 for any caller without archives:read_all — and the list
paths filter created_by_id == user.id. On a multi-user instance the tally
therefore reported archives restored while the person who owns them could
neither list nor open them.
Same shape as the deleted_at fix, and the same remedy: the collector records the
key next to deleted_at, the restore mirrors the printer_id/project_id pattern
exactly — one hoisted select(User.id), a membership test per row, an unknown id
coerced to None rather than failing the row, and one de-duplicated note. It is in
the overwrite setattr loop too, so overwrite keeps meaning "make local match the
backup". Additive on the backup side, so older backups still restore; they just
cannot know the owner.
Clearing the id is not silent-safe, so the note says what it costs: those
archives are visible only to users with archives:read_all until an admin
reassigns them.
Caveat recorded in a comment and raised in the PR, not decided here: this is the
one place the module reuses a raw backup id, against its own rule. Validating it
means a *stale* id clears rather than pointing somewhere wrong, but a live id
belonging to a different person on a different instance would still collide.
Collecting username and resolving on that would close it.
6 unit tests and 1 integration test that all fail against the parent commit,
plus 2 controls that pass either way — a backup with no created_by_id key still
restores, and a second operator still gets a 404.
maziggy