Alert when VMs or containers fail to autostart at boot
## Problem
A VM or container that fails to start during the boot autostart pass is swallowed into a single log line - no alert, no retry, nothing persisted. There is no alert class for VMs or containers anywhere in the tree, so the only signal is `/var/log/middlewared.log`. A user hit this with an autostart VM that failed to come up at boot and stayed down all day simply because nothing told them.
## Solution
- **Two aggregated one-shot alerts**, `VMAutostartFailedAlert` and `ContainerAutostartFailedAlert`, under a new `VIRTUALIZATION` alert category. Both use the `keys=[]` convention already used by the NFS/SMB/GPU aggregate alerts, so ten failed VMs produce one alert naming all ten rather than ten alerts.
- **Cleared at the start of every autostart pass and re-raised at the end** if anything failed, so the alert always describes the pass that just ran: a guest fixed between boots has its alert cleared, a still-broken one gets it re-raised. The clear is unconditional, so removing autostart from a broken guest also clears it.
- **The alert names the guests and points at the log for the why.** Error strings are deliberately kept out of the body - concatenating a few libvirt or validation errors produces something nobody can read. The reason is in the log instead, which is also why the two error logs here now pass `exc_info=True` like every one of their siblings, so the log has the full traceback rather than a bare `str(e)`.
Both HA failover entry points funnel into the same two functions, so they are covered without changes. The pool-unlock and pool-import autostart path swallows failures identically but is left alone for now - it needs per-path keys before it can share these alert classes, since `keys=[]` means a create from one path would wipe the other's alert.
Resolve migrated USB devices by their vendor and product ids
## Problem
A USB passthrough device is named after the port it is plugged into, which is what 25.10 stored. The 26.0 pre-releases built the same shape of name out of the bus and device number instead — an enumeration counter the kernel reissues on every replug — so those stored values name a port nobody chose.
The incus migration has the same preference the same way round: it resolves a device by the bus and device number the manifest carries and only falls back to the vendor and product ids when those are missing. It picks the counter that gets reused over the identity that survives a replug, and because it runs exactly once per machine and is never run again, a wrong row it writes stays wrong. There is no second run, no manifest retained afterwards, and no repair path.
This matters for a population that has not been hit yet: `maybe_migrate_legacy` first shipped in 26.0.0-BETA.1 and does not exist in 25.10, so every 25.10 system with incus containers still has `virt_global.pool` set and spends its single migration on its first boot of a 26.0 build.
## Solution
- **Only the vendor and product ids identify a device.** The bus and device number takes no part in resolution. A manifest device that does not carry both ids is dropped rather than guessed at, and the reason is logged.
- **Ids that no connected device carries are still stored as ids.** The device is merely unplugged; the row stays visible and editable rather than disappearing.
- **One device per row.** Rows written for a container claim what they point at, so two manifest devices can no longer land on one port or one pair of ids — which used to leave a container that could neither start nor be edited back. Where several identical devices are connected, each row takes the first one still free.
- **Devices pylibvirt cannot read are not candidates.** Entries marked unavailable are left out of the index, so a manifest no longer resolves onto one and then fails at start claiming the port is empty.
- **A device that cannot be resolved at start reads as an error, not a traceback.** pylibvirt's exception type is caught at both start call sites and reported as a `CallError`, which is what 25.10 did.
- **Every resolution is logged, not only the failures.** Nothing on the row records which identity it came from and the job log does not survive a restart, so this is the only thing that makes a wrong outcome reconstructable later.
- Also included: MAC normalization for NIC rows the incus migration writes (these go straight to the datastore, so `MACAddress` never sees them and the alembic pass has already run by then), and removal of `middlewared/utils/usb.py`, which had no callers and was a second implementation of the old naming scheme.
This is the same fix already on the release line, retyped against `ServiceContext`: master's `migrate.py` has no `ContainerService`, `migrate_devices` takes a `ContainerEntry` rather than a dict, and the udev scan is threaded down through `to_thread`/`run_coroutine` instead of `middleware.call`.
NAS-142151 / 26.0.0-RC.1 / Resolve migrated USB devices by their vendor and product ids (#19494)
## Problem
A USB passthrough device is named after the port it is plugged into,
which is what 25.10 stored. The 26.0 pre-releases built the same shape
of name out of the bus and device number instead — an enumeration
counter the kernel reissues on every replug — so those stored values
name a port nobody chose.
The incus migration has the same preference the same way round: it
resolves a device by the bus and device number the manifest carries and
only falls back to the vendor and product ids when those are missing. It
picks the counter that gets reused over the identity that survives a
replug, and because it runs exactly once per machine and is never run
again, a wrong row it writes stays wrong. There is no second run, no
manifest retained afterwards, and no repair path.
This matters for a population that has not been hit yet:
[33 lines not shown]
Fix stale-SID recovery test to match stable/26's call_sync-based restart
PR #19223 (call_sync2 migration) wasn't backported here, so
_recover_ad still restarts idmap via middleware.call_sync, not
call_sync2. Update the test to match, instead of pulling in the
unrelated migration.
Address review: Run dlm.remote_down's peer poll as a background task
It was blocking the HA connection thread for up to 60s, delaying
reconnect attempts and other disconnect callbacks.
Tolerate transient NTB blips before calling dlm.reset_active
remote_down now polls the DLM port for up to 60 seconds before ejecting
the peer, so short NTB packet loss events don't trigger a spurious
reset_active
Also raise scan_secs from the default 5 to 30 so the kernel DLM itself
doesn't start FORCEUNLOCK recovery during ~20s NTB blips.
NAS-142191 / 26.0.0-RC.1 / Don't swallow app upgrade failures in upgrade_impl (#19523)
## Problem
`upgrade_impl()` returns from inside a `finally` block, which discards
any exception raised by `app.pull_images_internal`. The `or
app['custom_app']` condition makes that branch unconditional for custom
apps, so a failed `docker compose pull` is reported to the user as a
successful upgrade: the job ends in SUCCESS with "App successfully
upgraded and redeployed" while the real error only lands in
/var/log/app_lifecycle.log.
The wrong status also hides the two steps that get skipped when the pull
raises, both sitting after `compose_action()` in
`pull_images_internal()`: the `clear_update_flag_for_tag` loop and
`app.redeploy`. So the app isn't recreated and the update flag stays
set, which is why the badge comes back.
## Solution
Clearing a stale alert after a partial pull is still worth doing, so the
[5 lines not shown]
NAS-142115 / 26.0.0-RC.1 / Add RAW data type for TDB databases (by anodos325) (#19492)
The TDB data types were added for early clustered SCALE, where the
contents had to be JSON serializable, and so binary values are
base64-encoded on insertion and decoded on read. We now call into the
native tdb tables directly and no longer need that intermediate step
except where a value still leaves middleware as JSON.
Add TDBDataType.RAW, which reads and writes bytes directly. The
conversion moves into a shared encode_value() so the local and clustered
write paths cannot disagree about it, and it now checks the value
against the data type. Together these remove a category of bug where the
encode and decode of a pair do not match.
Extend the tdb, passdb and group_mapping unit tests for the new data
type and the on-disk formats.
Original PR: https://github.com/truenas/middleware/pull/19490
[2 lines not shown]
Add RAW data type for TDB databases
The TDB data types were added for early clustered SCALE, where the
contents had to be JSON serializable, and so binary values are
base64-encoded on insertion and decoded on read. We now call into the
native tdb tables directly and no longer need that intermediate step
except where a value still leaves middleware as JSON.
Add TDBDataType.RAW, which reads and writes bytes directly. The
conversion moves into a shared encode_value() so the local and clustered
write paths cannot disagree about it, and it now checks the value
against the data type. Together these remove a category of bug where the
encode and decode of a pair do not match.
Extend the tdb, passdb and group_mapping unit tests for the new data
type and the on-disk formats.
(cherry picked from commit 0e6177daa43d2309e934a31a05a0de8001aaacda)
NAS-142059 / 26.0.0-RC.1 / Directory services cache fixes (by anodos325) (#19484)
insert_cache_entry() built the NAME_ key from the numeric id rather than
the name, so entries added by the lazy insertion path were only
reachable by id. A by-name lookup for such an entry missed the cache and
fell through to NSS plus idmap.synthetic_user. The bulk fill path was
unaffected, and query_cache_entries() reads only ID_-prefixed keys, so
enumeration results did not change.
DSCacheFill did not remove its temporary files when `fill_cache()`
raised `before _commit()`, so a failed fill can leave
`directory_service_cache_tmp_*` files behind in the cache `directory.
fill_cache()` documents NssError, WBCErr and job abort as reachable, and
nothing else removes those files. Cleanup now also covers a failure
partway through `__enter__`, which does not invoke `__exit__`.
expire_cache() now writes a timezone-aware UTC timestamp, matching
fill_cache() and check_cache_expired().
[12 lines not shown]
NAS-141754 / 26.0.0-RC.1 / Make ZFS deduplication incompatible with the PERFORMANCE tier (by anodos325) (#19483)
Deduplicated datasets are excluded from tiering: they report a null
tier, and dataset_set_tier and rewrite_job_create reject them.
Enabling dedup (ON/VERIFY) is refused only when a dataset's data is on
the SPECIAL vdev, i.e. the PERFORMANCE tier (special_small_blocks > 0).
REGULAR datasets, volumes, and pools without a SPECIAL vdev may be
deduplicated freely.
Original PR: https://github.com/truenas/middleware/pull/19336
---------
Co-authored-by: Logan Cary <logan.cary at ixsystems.com>
Resolve migrated USB devices by their vendor and product ids
## Problem
A USB passthrough device is named after the port it is plugged into, which is what 25.10 stored. The 26.0 pre-releases built the same shape of name out of the bus and device number instead, and migration 0021 was added to convert those. It cannot do that job: nothing on either device table records which build wrote a row, so it has no way to tell a device-number value from a correct one and rewrites both. A container row written by any current build, or a 25.10 VM row whose device happened to be unplugged at the upgrade boot, is silently repointed at whatever now holds those digits. No shipped release ever wrote a device-number name, so everything the migration could fix is pre-release while everything it can break is not.
The incus migration had the same preference the same way round: it resolved a device by the bus and device number the manifest carried and fell back to the vendor and product ids only when those were missing, choosing the counter the kernel reissues over the identity that survives a replug. It runs once per machine and is never run again, so a wrong row it writes stays wrong.
## Solution
- **Migration 0021 is removed.** Without it every upgrade path is already correct, because the names 25.10 stored are the names the current code produces.
- **Only the vendor and product ids identify a device.** The bus and device number takes no part in resolution. A manifest device that does not carry both ids is dropped rather than guessed at, and the reason is logged: this migration cannot be repeated, and a row built from a stale address is a wrong row that nobody afterwards can tell is wrong.
- **Ids that no connected device carries are still stored as ids.** The device is merely unplugged, and a row saying what the user asked for stays visible and editable rather than disappearing.
- **One device per row.** Rows written for a container claim what they point at, so two manifest devices can no longer land on one port or one pair of ids, which used to leave a container that could neither start nor be edited back. Where several identical devices are connected, each row takes the first one still free.
- **A device that cannot be resolved at start reads as an error rather than a traceback.** pylibvirt's own exception type is caught at both start call sites and reported as a `CallError`, which is what 25.10 did.
- Every resolution is logged, not only the failures. Nothing on the row records which identity it came from, the job log does not survive a restart, and the migration never runs again, so this is the only thing that makes a wrong outcome reconstructable later.
Devices whose manifest carries no usable vendor and product ids are now dropped where they previously became a row built from the recorded bus and device number. That is deliberate and it is permanent: there is no second run and no repair path. In practice the affected set is small, since anything selected through the 25.10 UI carried all four fields.
Resolve migrated USB devices by their vendor and product ids
## Problem
A USB passthrough device is named after the port it is plugged into, which is what 25.10 stored. The 26.0 pre-releases built the same shape of name out of the bus and device number instead, and migration 0021 was added to convert those. It cannot do that job: nothing on either device table records which build wrote a row, so it has no way to tell a device-number value from a correct one and rewrites both. A container row written by any current build, or a 25.10 VM row whose device happened to be unplugged at the upgrade boot, is silently repointed at whatever now holds those digits. No shipped release ever wrote a device-number name, so everything the migration could fix is pre-release while everything it can break is not.
The incus migration had the same preference the same way round: it resolved a device by the bus and device number the manifest carried and fell back to the vendor and product ids only when those were missing, choosing the counter the kernel reissues over the identity that survives a replug. It runs once per machine and is never run again, so a wrong row it writes stays wrong.
## Solution
- **Migration 0021 is removed.** Without it every upgrade path is already correct, because the names 25.10 stored are the names the current code produces.
- **Only the vendor and product ids identify a device.** The bus and device number takes no part in resolution. A manifest device that does not carry both ids is dropped rather than guessed at, and the reason is logged: this migration cannot be repeated, and a row built from a stale address is a wrong row that nobody afterwards can tell is wrong.
- **Ids that no connected device carries are still stored as ids.** The device is merely unplugged, and a row saying what the user asked for stays visible and editable rather than disappearing.
- **One device per row.** Rows written for a container claim what they point at, so two manifest devices can no longer land on one port or one pair of ids, which used to leave a container that could neither start nor be edited back. Where several identical devices are connected, each row takes the first one still free.
- **A device that cannot be resolved at start reads as an error rather than a traceback.** pylibvirt's own exception type is caught at both start call sites and reported as a `CallError`, which is what 25.10 did.
- Every resolution is logged, not only the failures. Nothing on the row records which identity it came from, the job log does not survive a restart, and the migration never runs again, so this is the only thing that makes a wrong outcome reconstructable later.
Devices whose manifest carries no usable vendor and product ids are now dropped where they previously became a row built from the recorded bus and device number. That is deliberate and it is permanent: there is no second run and no repair path. In practice the affected set is small, since anything selected through the 25.10 UI carried all four fields.
Make ZFS deduplication incompatible with the PERFORMANCE tier
Deduplicated datasets are excluded from tiering: they report a null
tier, and dataset_set_tier and rewrite_job_create reject them.
Enabling dedup (ON/VERIFY) is refused only when a dataset's data is on
the SPECIAL vdev, i.e. the PERFORMANCE tier (special_small_blocks > 0).
REGULAR datasets, volumes, and pools without a SPECIAL vdev may be
deduplicated freely.
Point stored USB passthrough devices at their physical port
## Problem
A USB device name is built from the physical port it is plugged into, which is what libvirt has always called them and what 25.10 stored. The 26.0 pre-releases built the same shape of name out of the bus and device number instead, and a device number is an enumeration counter the kernel reissues on every replug, so those stored values name a port nobody chose and resolve to whatever device happens to hold the digits.
The Incus migration minted those names straight from the manifest without checking that anything was there, and prefixed `0x` unconditionally onto vendor and product ids, turning one that already carried a prefix into `0x0x046d` — a value that matches no device and that the schema accepted, since it only asks for a `0x` prefix.
## Solution
- **A migration that converts stored names to the port they refer to.** Only live hardware can map a device number back to a port, which is why this is a middleware migration rather than an alembic one: it runs on the machine the devices are attached to, while they are still attached. `container_device` was created in 26.0 and has never existed earlier, so every value in it is converted; a `vm_device` value may predate 26.0, so it is kept whenever it already names a port that something is plugged into.
- **Anything that resolves to nothing is left alone and logged.** The device is unplugged, or the database arrived from another machine, and either way there is nothing here to point it at. The stored name then fails at start with `USB device <name> not found` rather than quietly passing through whichever device the digits land on.
- **The Incus migration resolves bus and device number to a port while the device is still plugged in**, which is the only point at which that is possible, and skips a pair that lands on a hub.
Keep two USB devices from being pointed at one port
## Problem
The USB port migration resolved each stored bus and device number to the port currently holding it, but never checked whether another device was already pointing there. A row carrying a device number and a row that already named that port both ended up on it, and nothing downstream objects: the uniqueness check only runs when a device is created or updated. Which of the two rows won also depended on the order the rows happened to come back in.
## Solution
- **A claim ledger, scoped per instance.** Every row is resolved before anything is written, and rows that keep their stored value claim their port first, so a rename landing on a port another device of the same instance already holds is left alone and logged rather than applied. Scoping to the instance rather than the table is deliberate: two VMs configured for one dongle is valid config, they simply cannot both be running.
- **One udev scan for the whole run.** `usb_device_names_by_bus_and_devnum` builds the bus and device number map in a single pass, so every row is resolved against the same view of the machine instead of one scan per row.
- **A write that fails no longer takes the rest down with it.** A migration that raises is not recorded and starts over on the next boot, by which time the kernel has reissued the very device numbers the rows that did get written were resolved from.
Log lines now name the instance, so an admin reading them can tell which VM or container lost a device without going to the database for it.
`middlewared/utils/usb.py` goes too: it had no callers anywhere and was a second implementation of the naming scheme this work replaced, waiting for someone to wire it up.
NAS-142099 / 27.0.0-BETA.1 / Remove unreferenced private methods from disk plugins (#19486)
Removes four `@private` methods in the `disk` namespace that have no
callers left in middleware, the test suites, or webui.
`disk.identifier_to_device` lost its last caller in June 2022 when
`disk.sync_all` was optimized
([NAS-116484](https://ixsystems.atlassian.net/browse/NAS-116484)); an
equivalent lookup lives in `disk_/sync.py` and is the one actually used.
The other three are older leftovers: `disk.get_partition_uuid_from_name`
maps FreeBSD partition-type names to GUIDs, `disk.get_efi_part_type`
returns a constant nothing reads, and
`disk.update_partition_table_quick` still documents itself as "Used by
'wipe'" although `wipe.py` no longer calls it.
Also drops the `label` identifier type from the
`disk.device_to_identifier` docstring.
Don't swallow app upgrade failures in upgrade_impl
## Problem
`upgrade_impl()` returns from inside a `finally` block, which discards any exception raised by `app.pull_images_internal`. The `or app['custom_app']` condition makes that branch unconditional for custom apps, so a failed `docker compose pull` is reported to the user as a successful upgrade: the job ends in SUCCESS with "App successfully upgraded and redeployed" while the real error only lands in /var/log/app_lifecycle.log.
The wrong status also hides the two steps that get skipped when the pull raises, both sitting after `compose_action()` in `pull_images_internal()`: the `clear_update_flag_for_tag` loop and `app.redeploy`. So the app isn't recreated and the update flag stays set, which is why the badge comes back.
## Solution
Clearing a stale alert after a partial pull is still worth doing, so the state refresh stays in the `finally` and only the early return and its progress update move out. The success path behaves exactly as before.
Adds unit tests for both the failure and the success path.
NAS-142191 / 27.0.0-BETA.1 / Don't swallow app upgrade failures in upgrade_impl (#19506)
## Problem
`upgrade_impl()` returns from inside a `finally` block, which discards
any exception raised by `pull_images_internal()`. For custom apps the
`or app.custom_app` condition makes that branch unconditional, so a
failed `docker compose pull` gets reported to the user as a successful
upgrade.
What you see: the Update job finishes in about a second with state
SUCCESS and progress "App successfully upgraded and redeployed", then
the update badge reappears. The real error only lands in
`/var/log/app_lifecycle.log`.
The wrong status also hides two steps that get skipped when the pull
raises, both sitting after `compose_action()` in
`pull_images_internal()`: the `clear_update_flag_for_tag` loop and
`app.redeploy`. So the app isn't recreated and the update flag stays
[29 lines not shown]
Only report a missing license on HA capable systems
This commit fixes an issue where an unlicensed R-series or Z-series started getting told to contact support about a license it was never sold. Those chassis are appliances so they satisfy the hardware rule, but they cannot fail over, and on 26 the product_type disjunct meant a system with no license only reached this alert when it was HA capable. HA capability is a chassis probe the applicability axes deliberately do not carry, so the test sits in check_sync rather than in applies_to.
Run the local account expiration alert everywhere
This commit fixes an issue where the local account expiration alert stopped running on Minis even though they can still turn on password aging: max_password_age sits behind the STIG entitlement, whose vector grants on the key column of either hardware side, and that key is injected into every legacy licence. The source is also the admin lockout recovery, since it regenerates shadow a day before the last full admin password ages out, so excluding a system that can arm password aging leaves it with no way back in short of a reboot.
check() already returns early unless max_password_age is set, so that early return is the applicability test and the rule is dropped rather than widened. On a system that cannot set the option the daily cost is one config read.
Remove the broken SATA DOM wear alert
This commit adds changes to delete the SATA DOM wear alert source and its two classes. The source calls disk.sata_dom_lifetime_left, which was removed in 25.10 and has no implementation anywhere, so it raises on every run on any M or Z series chassis and produces nothing but alert source failures.
Alerts already persisted on existing systems clear on the next start through the stale source check in initialize, which runs before the policies are built, so nothing is announced as cleared. No migration is needed, matching how every previous alert source removal was handled.
Remove the product_type gate from alerts
This commit adds changes to delete `products` from every alert declaration now that the hardware and license axes carry the gating, along with the `product_types` field it fed in `alert.list_categories` and the unused `AlertService.product_type` helper. The applicability matrix loses its old-versus-new comparison and becomes a frozen inventory of what each declaration covers; the declared answers are unchanged from the previous commit, cell for cell.
The API field is removed outright rather than deprecated because nothing consumes it - it exists in no earlier API version and has no client in the WebUI, api_client or midcli, so the downgrade adapter's strip loop is already a no-op here. Forward-porting this to master will need a `to_previous` on `AlertCategoryClass`, because there v26_0_0 is frozen with the field required and the version adapter only backfills fields that are not required.
Move alert gating onto hardware and license axes
This commit adds changes to give every declaration under alert/source its own applicability rule on the decoupled hardware and license axes, and points the enforcement sites in plugins/alert.py at those rules instead of system.product_type. The products lines stay for now because the API field still reads them. HA classes take a hardware applies_to alongside listed_when on purpose, so on iX hardware whose HA licence is absent or lapsed they leave the settings catalogue without being silenced, and a source that no longer applies now has its stored alerts dropped rather than stranding them undismissable. The two scheduled-reboot classes are the exception: they are gated on the HA licence itself, so a system without one stops seeing them at all.
sata_dom_wear is knowingly shipped in a broken state: its source calls disk.sata_dom_lifetime_left, which was removed in March 2025, so appliances will see the check fail. That is a deliberate, signed-off decision and not an oversight.
The checked-in matrix under pytest/unit/alert/golden spells out old versus new applicability per population for every declaration, and its test regenerates and diffs it so no population change lands unread.
Name alert applicability rules and detect black-holed alerts
This commit adds changes to give the alert applicability engine one name per population instead of thirty open-coded rule constructions, and to catch alerts that are created but can never be displayed.
The rules alert declarations gate on now live in a vocabulary module next to the engine, so TRUENAS_HARDWARE is defined once rather than built at eighteen sites in two competing conventions. LicenseRule collapses into EntitlementRule, which asks the entitlement policy about any feature rather than the two the old enum could spell, so STIG and everything after it is a one-line declaration. AllOf joins AnyOf in the algebra, listed_when becomes listed_only_when to say that it narrows, and AlertFacts is gone in favour of the identical EntitlementFacts that applies() was converting to anyway.
A source whose rule is satisfied where its class's rule is not creates alerts that are stored and never shown. Nothing checked for that, and since the failover declarations deliberately straddle the two axes it is no longer something a reviewer can spot by eye. A test now infers the source to class relation out of check() and asserts the implication across every population, with a runtime guard logging the cases static analysis cannot see. It found four pre-existing ones in memory_errors and sensors, which are fixed by narrowing the sources so nothing user visible moves.
One behaviour change worth a release note: failover_related was gating enclosure_status on an HA licence, which is a licensing predicate in front of a hardware question, so unlicensed appliances will now report enclosure faults they were previously silent about. The flag is renamed post_failover_blackout to name what it actually contributes.
Keep the FIPS misconfiguration alert on licensed Minis and whiteboxes
This commit fixes an issue where the FIPS misconfiguration alert stopped running on licensed Minis and licensed whiteboxes even though those systems can still enable FIPS: the STIG vector is key-only on both hardware sides and STIG is injected into every legacy licence, so they are entitled while TRUENAS_HARDWARE excluded them by construction.
EXPECTED_TO_BE_LICENSED is a strict superset of both the old product_type gate and the hardware gate, so nothing loses coverage, unlicensed appliances keep the check that catches FIPS being active when it was never configured, and licensed whiteboxes that could never see this alert are now covered too.
Type alert applicability against AlertSource and AlertClass
This commit adds changes to drop the Declaration protocols in favour of TYPE_CHECKING imports of AlertSource and AlertClass, which is what review asked for, and to tighten the surface while we are in there: rule_name is no longer re-exported now that declaration_rule_name is the only entry point production goes through, and the Rule docstring keeps just the part that constrains how you write one, which is to name it so the black-hole diagnostic has something to report.
Dropping the protocols makes alert.applicability depend on alert.base, and import-linter counts TYPE_CHECKING imports, so that broke the package layering contract: engine reached vocabulary and snapshot through alert.base importing the whole package. alert/base.py now takes Rule from the engine module directly, which is the only thing it ever wanted, and the contract holds without an ignore.