Extract a transport-agnostic network discovery backend by rtibblesbot · Pull Request #15028 · learningequality/kolibri

rtibblesbot · 2026-07-17T20:18:12Z

Summary

Network discovery — mDNS broadcast plus peer listening — was welded to zeroconf inside KolibriBroadcast and only ran inside the server ProcessBus, so no platform could supply an alternative transport or run discovery elsewhere. This extracts a transport-agnostic NetworkDiscoveryBackend and a single-registration NetworkDiscoveryHook transport interface, with the zeroconf implementation moved behind that hook into its own zeroconf_transport module and registered as the default. The backend owns the broadcast-id lifecycle, event bus, dedup cache, and listener dispatch, and can be constructed and started/stopped without a KolibriProcessBus. Server broadcast and discovery behaviour is unchanged.

One operator-visible change: the peer log lines drop the transport name — Kolibri instance 'x' joined zeroconf network becomes joined the network, same for the update and leave lines, as the backend that logs them no longer knows the transport.

References

Fixes #14996.

Known-failing checks, unrelated to this branch: Build Raspberry Pi Image (apt.learningequality.org has no published Release file — #15149) and Build WHL file / WHL smoke tests (intermittent IncompleteRead fetching cffi wheels in staticdeps-cext).

Reviewer guidance

zeroconf_transport.py:330 — the transport is a process-lifetime singleton now, so a browser event can outlive the backend that started it (Zeroconf.close() joins the engine and reaper, not the browser threads). Check _is_current, the no-op callbacks (:319) and the _build_instance self-guard (:496) drop every stale dispatch rather than raising or landing a previous broadcast's peer in the next one.
zeroconf_transport.py:253 — the transport calls on_rebind before zeroconf.update_interfaces(), so a peer the rebind rediscovers is enqueued under the incoming broadcast id and survives the reset that follows.
broadcast.py:474 — the other_instances cache is keyed by service name and on_remove(name) resolves the cached instance (a removed service can't be re-queried); confirm reset_broadcasting() there lets a peer leave→rejoin re-publish an ADD.
broadcast.py:337 — start_broadcast registers before it listens, so the transport stores its own instance before any peer event fires; check this ordering contract holds for an override transport.
zeroconf_transport.py:177 — the .local aliases stayed with the transport, which carries its own event bus and subscribes LocalHostnameListener lazily rather than in __init__: the plugin registry instantiates the transport in every process at ready(), and an eager import pulls discovery.tasks and its models in with it.
kolibri/utils/server/__init__.py:321 — run() calls update_broadcast() every tick with no addresses-changed check, and the transport detects the change internally; confirm the LISTEN_ADDRESS != "0.0.0.0" gate (dummy monitor, frequency=-1) still skips dynamic updates for specific-address deployments.

AI usage

Used Claude Code to implement the extraction from a human-reviewed implementation plan, following the existing overridable-default hook mechanism. Verified with the discovery and server test suites and ruff.

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Ran pre-flight CI checks (lint, format, tests) and verified all pass
Rebased onto the target branch and resolved any conflicts
Reorganized commit history into clean, logical commits
Audited the diff to ensure only issue-relevant files are changed
Built PR body from the repository's PR template with evidence blocks

Deviations from the issue spec

Criterion	Why not
`NetworkDiscoveryHook` defines `update(instance)`	Takes `update(instance, on_rebind)`. The backend has to cycle its broadcast id before the transport rebinds, so a peer the rebind rediscovers is enqueued under the incoming id.
`start_listening(on_add, on_update, on_remove)`	Takes a fourth callback, `is_known(name)`. Without it the transport resolves a peer before the backend consults its cache, so a repeat ADD for a known peer costs a 10s-timeout query whose result `on_add` discards.

	🟡 Waiting for feedback Last updated: 2026-08-18 01:47 UTC

rtibbles

Just one comment so far.

rtibbles · 2026-07-17T21:11:40Z

        }


+@register_hook(as_default=True)


I think this could live in a kolibri_plugin.py inside discovery instead of going into the core.

Good call — moved the registration into a new kolibri/core/discovery/kolibri_plugin.py, so it now lives alongside the transport it registers rather than in core. register_non_plugins loads it automatically since kolibri.core.discovery is an installed app. Done in 9fd6518.

github-actions · 2026-07-17T21:52:29Z

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.20.0a2.dev64_g6c2d31349.pex
Windows Installer (EXE)	kolibri-setup-0.20.0a2.dev64+g6c2d31349-unsigned.exe
Debian Package	kolibri_0.20.0a2.dev64+g6c2d31349-0ubuntu1_all.deb
Debian Package	kolibri-server_0.20.0~a2.dev64+g6c2d31349-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.20.0a2.dev64+g6c2d31349.dmg
Android Package (APK)	kolibri-0.20.0a2.dev64+g6c2d31349-debug.apk
Raspberry Pi Image	kolibri-pi-image-0.20.0a2.dev64+g6c2d31349.zip
TAR file	kolibri-0.20.0a2.dev64+g6c2d31349.tar.gz
WHL file	kolibri-0.20.0a2.dev64+g6c2d31349-py3-none-any.whl

Smoke test screenshot

Download screenshot

rtibbles

Not behaviour-preserving: the transport resolves before the backend checks its cache, and the guarding tests were deleted, not ported.

rtibbles · 2026-08-16T01:35:11Z

+
+    def _added(self, name):
+        logger.debug("Received ADD event for Zeroconf service: {}".format(name))
+        instance = self._resolve_instance(name)


Resolves before the backend checks its cache, so a repeat ADD for a known peer costs a 10s-timeout query on_add discards. Deleted test_add_service__cached asserted otherwise; add an is_known(name) check.

Added NetworkDiscoveryBackend.is_known(name), passed to start_listening as a fourth callback; _added returns early when it reports True. Covered by test_added_event_for_known_service_skips_query and test_is_known_only_for_cached_broadcasting_peer (the cached-but-not-broadcasting case still queries). 6f30a9d

rtibbles · 2026-08-16T01:35:11Z

+        """
+        if instance.is_self:
            return
+        existing = self.other_instances.get(instance.name)


Dedup moved here verbatim; its eight test_update_service__* tests were dropped. Port them.

Ported as test_on_update__* against backend.on_update in test_network_backend.py, plus test_on_update_ignores_self_instance. 386cd40

rtibbles · 2026-08-16T01:35:11Z

+        """Stops advertising our instance and discovering peers."""
+        # very important to publish the event first, to avoid race conditions
+        self.events.publish(EVENT_UNREGISTER_INSTANCE, self.instance)
+        self.transport.unregister()


self.transport is None if start_broadcast() raised after self.backend was set — STOP and each tick then AttributeError. Guard it.

Guarded in both stop_broadcast and update_broadcast. 32d7b46

rtibbles · 2026-08-16T01:35:11Z

+            LocalHostnameListener,
+        )
+
+        self.add_listener(LocalHostnameListener)


Hooks instantiate at ready(), pulling discovery.tasks and its models into every process. Subscribe in start_listening().

Moved to a lazy _ensure_local_hostname_listener(), called from start_listening and register. Only start_listening would drop the local names published during register (the backend registers first), so sync_local_hostnames would never run until the next renew. ff6300c

rtibbles · 2026-08-16T01:35:11Z

+        # The transport detects an address change internally and rebinds, so we
+        # update unconditionally on each tick; the backend cycles its broadcast
+        # id and resets connection state when the transport reports a rebind.
+        self.backend.update_broadcast()


Rebinds now log only at DEBUG. Restore the INFO line.

Restored, but in ZeroconfNetworkDiscovery.update() — the plugin no longer detects the change, the transport does. Same message, same point in the flow. 93a36b3

rtibbles · 2026-08-16T01:35:11Z

+        self.backend = NetworkDiscoveryBackend(instance=self.instance)
+
+    def tearDown(self):
+        NetworkDiscoveryHook._registered_hooks = {}


{} discards the real default transport. Save in setUp, restore here.

Saved in setUp, restored in tearDown. b3a24d3

rtibbles · 2026-08-16T01:35:11Z

+        listener.assert_not_called()
+        self.assertEqual(self.backend.other_instances, {})
+
+    def test_on_update_publishes_update(self):


Only on_update coverage left, and it skips the dedup; _build_instance's is_self is mocked everywhere. Restore both.

Dedup ported as the test_on_update__* cases (386cd40); _build_instance now has test_build_instance / test_build_instance__self in ZeroconfNetworkDiscoveryTestCase for the real is_self computation.

rtibbles · 2026-08-16T01:35:11Z

+            NetworkDiscoveryHook.registered_hook, ZeroconfNetworkDiscovery
+        )
+    finally:
+        NetworkDiscoveryHook._registered_hooks = {}


Same clobber. Restore the previous dict.

Fixture and test_default_transport_is_zeroconf both save the registry and restore it. b3a24d3

rtibbles

Guards deleted, not just their tests: the three lifecycle no-ops are gone, and six of the deleted cases still have no equivalent.

rtibbles · 2026-08-16T03:29:28Z

-        ]
+        self.transport = None

    def start_broadcast(self):


Develop returned early here when already broadcasting; the guard and test_start_broadcast__already_broadcasting both went. Restore it, or say why a second start is safe.

Restored, keyed on transport (the backend's equivalent of develop's zeroconf is not None); stop_broadcast clears it so a restart still works. test_start_broadcast__already_broadcasting asserts the transport is not re-registered.

rtibbles · 2026-08-16T03:29:28Z

-            logger.error("Zeroconf service is not broadcasting!")
+        """Stops advertising our instance and discovering peers."""
+        # very important to publish the event first, to avoid race conditions
+        self.events.publish(EVENT_UNREGISTER_INSTANCE, self.instance)


Published unconditionally, where develop returned early when not broadcasting. start_broadcast() raising RuntimeError still leaves ZeroConfPlugin.backend set, so STOP reaches here and reset_connection_states(<never-broadcast id>) deletes every DynamicNetworkLocation.

Restored — stop_broadcast now returns before publishing when transport is None. test_stop_broadcast__never_started drives exactly your scenario: start_broadcast() raises, then stop_broadcast() must publish no UNREGISTER.

rtibbles · 2026-08-16T03:29:28Z

+        if instance.is_self:
+            return
+        existing = self.other_instances.get(instance.name)
+        if existing is not None and existing.is_broadcasting:


Untested — test_leave_rejoin_republishes_add passes with or without it, and is_known only covers the transport-side skip. This is the guard test_add_service__cached held.

Ported as test_on_add__cached_broadcasting_peer_ignored: a re-Added peer already cached and broadcasting publishes nothing and leaves the cached instance in place.

rtibbles · 2026-08-16T03:29:28Z

+            longer be queried, so we resolve the cached instance by name.
+        """
+        instance = self.other_instances.get(name)
+        if instance is not None and not instance.is_self and instance.is_broadcasting:


test_remove_service__is_self and test_remove_service__not_found weren't ported; neither guard is exercised.

Ported both as test_on_remove__is_self and test_on_remove__not_found.

rtibbles · 2026-08-16T03:29:28Z

@@ -577,9 +668,6 @@ def register(self):
            if i > SERVICE_RENAME_ATTEMPTS:


test_register__rename_fail — which patched SERVICE_RENAME_ATTEMPTS to 0 — wasn't ported, so the raise is untested.

Ported as test_register_rename_gives_up, with SERVICE_RENAME_ATTEMPTS patched to 0.

rtibbles · 2026-08-16T03:29:28Z

+
+    def unregister(self):
+        """Stops advertising our instance and drops our `.local` aliases."""
        if not self.is_broadcasting:


test_unregister__not_broadcasting and test_renew__not_broadcasting weren't ported; both early returns are untested.

Ported both as test_renew__not_broadcasting and test_unregister__not_broadcasting.

rtibbles · 2026-08-16T03:29:28Z

+        # discard the result anyway
+        if self._is_known(name):
+            return
+        instance = self._resolve_instance(name)


Only the Updated variant was ported (test_updated_event_missing_service_invokes_on_remove). test_add_service__not_found has no equivalent.

Ported as test_added_event_missing_service_dispatches_nothing: neither on_add nor on_remove fires.

rtibbles · 2026-08-16T03:29:28Z

+
+    def test_start_broadcast_without_transport_raises(self):
+        with self.assertRaises(RuntimeError):
+            self.backend.start_broadcast()


The transport-None guard added for the earlier comment ships with no test. Cover stop_broadcast/update_broadcast after start_broadcast raises.

Covered by test_stop_broadcast__never_started and test_update_broadcast__never_started, both starting from a start_broadcast() that raised.

rtibbles

Everything from the last two rounds checks out. What's left is the rapid start/stop hardening from dba1c0a: a teardown racing a start used to no-op, and now raises.

rtibbles · 2026-08-16T04:14:31Z

+        # register our own instance before we start discovering peers, so the
+        # transport has stored our instance before any peer event fires
+        self.transport.register(self.instance)
+        self.transport.start_listening(


dba1c0a guarded the browser attach with if self.zeroconf is not None for a stop_broadcast() landing mid-start. That's gone: stop_broadcast() nulls transport, so this line raises AttributeError — and if it didn't, start_listening would reopen Zeroconf with a browser nothing closes.

Guarded: start_broadcast returns without listening if transport is None after register(). Covered by test_start_broadcast__stopped_while_registering.

rtibbles · 2026-08-16T04:14:32Z

+        # singleton doesn't keep the old backend (and its buses/instance) alive
+        self.instance = None
+        self._on_add = None
+        self._on_update = None


Zeroconf.close() joins the engine and reaper, not the browser threads, so a handler blocked in the 10s get_service_info resumes after this and calls None. Develop absorbed it — the same event reached _get_service_info, which returned None when not broadcasting.

_handle_service_change now drops any event whose zeroconf is not the one we are listening on, and _resolve_instance re-checks after the query returns — so a handler that was blocked in it never reaches the nulled callbacks. test_added_event_stopped_during_query_dispatches_nothing / test_updated_event_stopped_during_query_dispatches_nothing.

rtibbles · 2026-08-16T04:14:32Z

+        self._on_remove = None
+        self._is_known = None
+
+    def _handle_service_change(self, zeroconf, service_type, name, state_change):


_is_known(name) runs before that _get_service_info check, so it no longer absorbs anything. And the transport is a process-lifetime singleton now: a stale handler closes over self, whose _on_add a later start_listening has repointed at the new backend, so a peer from the previous broadcast lands in the next one. if not self.is_broadcasting: return at the top here covers that and the reopened-Zeroconf case above.

Same guard, keyed on the zeroconf the browser hands the handler rather than is_broadcasting: a reopened Zeroconf is a different object, so an event from the previous broadcast is dropped instead of reaching the new backend. test_event_queued_before_stop_listening_is_dropped, test_event_from_a_previous_listening_session_is_dropped.

rtibbles · 2026-08-16T04:14:32Z

+            raise RuntimeError("No network discovery transport registered")
+        # very important to publish the event first, to avoid race conditions, as listeners
+        # could rely on register event happening before other network events
+        self.events.publish(EVENT_REGISTER_INSTANCE, self.instance)


Develop published this after the rename loop settled zeroconf_id, and only for a register that then succeeded. Nothing reads zeroconf_id off the event today, so this is a lost contract rather than a bug.

Contract restored: the publish moved below transport.register(self.instance), which settles zeroconf_id via to_service_info and raises on a name conflict. Still ahead of start_listening, so REGISTER precedes any peer event. test_start_broadcast_publishes_register_after_the_transport_registered.

rtibbles

One left, same family as the last round: a browser event racing the teardown raises where develop absorbed it.

Also before merge — rebase onto develop (the base is a month old, though nothing has touched discovery/ or utils/server/ since), and fold the review-response commits back into the extraction commits.

rtibbles · 2026-08-16T04:43:10Z

+        if self.zeroconf is not None:
+            self.zeroconf.close()
+            self.zeroconf = None
+        # drop references into the finished backend so this process-lifetime
+        # singleton doesn't keep the old backend (and its buses/instance) alive
+        self.instance = None
+        self._on_add = None
+        self._on_update = None
+        self._on_remove = None
+        self._is_known = None


A handler that passed _is_current while Zeroconf.close() was still joining reaches self._is_known(name) once this returns: TypeError: 'NoneType' object is not callable. Same for _on_remove, dispatched straight from the handler. Develop absorbed it — the event reached _get_service_info, which returned None when not broadcasting. Reordering the nulls only moves the window; no-op defaults (_is_known returning False) close it and leave the existing checks to drop the event.

Done — _on_add/_on_update/_on_remove default to a no-op and _is_known to returning False, set in __init__ and restored by stop_listening, so a dispatch racing the teardown falls through to the existing _is_current checks. test_event_dispatched_across_a_stop_is_dropped_not_raised covers it; without the defaults it fails with exactly that TypeError.

Also rebased onto develop and folded the review-response commits back into the extraction commit.

rtibbles

One left, same family: the no-op callbacks cover the dispatch, not the instance the argument is built from.

rtibbles · 2026-08-16T05:09:02Z

+        # the query blocks for up to 10s, ample time for a `stop_listening`
+        if not self._is_current(zeroconf) or service_info is None:
+            return
+        self._on_add(self._build_instance(service_info))


_build_instance derefs self.instance, which stop_listening() nulls, and the argument is built before the no-op _on_add is reached — so a handler that passed the check above and then lost the race raises AttributeError: 'NoneType' object has no attribute 'zeroconf_id'. Same for _updated. _is_own_service already takes that guard; develop's instance was never nulled.

Guarded — _build_instance treats a nulled self.instance as "nothing is our own service", covered by test_instance_built_after_a_stop_is_not_our_own.

rtibbles

Last round's _build_instance comment is still open. One more ordering regression in the rebind path, plus two gaps.

Separately, the reviewer guidance in the PR body still describes the pre-review shape: the line refs have drifted, and none of the three bullets covers the stale-event handling that is now most of the change.

rtibbles · 2026-08-16T05:45:04Z


-        # when interfaces is being updated, pass along to Zeroconf so it can bind to them
-        if interfaces is not None:
+        if rebound:


Develop bumped the id and published UNREGISTER before zeroconf.update_interfaces(); both now follow it. A peer rediscovered during the rebind is enqueued under the outgoing id, and the reset_connection_states(new_id) that follows deletes its DynamicNetworkLocation — it only returns on that peer's next TTL update, since on_add won't re-fire for a cached broadcasting peer. Wants the transport to report the rebind before performing it. test_update_broadcast_rebound_cycles_id_and_unregisters asserts neither ordering.

update() now takes an on_rebind callback and calls it before zeroconf.update_interfaces(), restoring develop's order: id bumped and UNREGISTER published while still bound to the outgoing interfaces. Return value dropped. test_update_broadcast_rebound_cycles_id_and_unregisters asserts both the id at publish time and update → publish → rebind; test_update_reports_the_rebind_before_rebinding asserts it transport-side.

rtibbles · 2026-08-16T05:45:05Z

+        elif state_change is ServiceStateChange.Removed:
+            # a removed service can no longer be queried, so we hand the name to
+            # the backend, which resolves the cached instance
+            self._on_remove(name)


Develop logged a debug line for all three state changes; Added and Updated kept theirs.

Restored the REMOVE debug line.

rtibbles · 2026-08-16T05:45:05Z

-        broadcast.stop_broadcast.assert_called_once()
+        backend.stop_broadcast.assert_called_once()
+
+    def test_monitor_tick_before_backend_created_is_noop(self):


Covers only the backend is None guard. Nothing asserts that a tick with a backend calls update_broadcast().

Added test_monitor_tick_updates_the_broadcast: a tick with a backend calls update_broadcast() with no arguments.

rtibbles

Behaviour matches 15b0bc1 now. Left: a deleted test file, a new fatal failure mode, uneven teardown state.

rtibbles · 2026-08-16T07:20:57Z

-
-    @mock.patch(LOCAL_HOSTNAMES_MODULE + "sync_local_hostnames.enqueue")
-    def test_update_local_names(self, mock_enqueue):
-        hostnames = ["kolibri.local", "tonyslaptop.local"]


Deleted, not ported. The enqueue survives via test_register__local_names_queued_for_persistence; restore this against the transport or say why it's redundant.

Redundant: the file held one test, asserting LocalHostnameListener.update_local_names(hostnames) enqueues sync_local_hostnames(args=(hostnames,)) on a directly-constructed listener. test_register__local_names_queued_for_persistence asserts the same call, reached through the transport bus the transport subscribes the listener to, and additionally pins that nothing is enqueued before register. Restoring it would re-test one callback in isolation behind a path already covered end to end.

rtibbles · 2026-08-16T07:20:58Z

-    def local_hostnames(self):
+        self.transport = NetworkDiscoveryHook.registered_hook
+        if self.transport is None:
+            raise RuntimeError("No network discovery transport registered")


An unregistered transport is now fatal to RUN; develop only lost discovery. Unreachable today, but a platform whose override fails to register kills server startup. Log and return.

Logs and returns now — transport stays None, so STOP and each tick no-op through the existing guards. test_start_broadcast_without_transport_loses_discovery_only covers it.

rtibbles · 2026-08-16T07:20:58Z

            return
+        self.other_instances[instance.name] = instance
+        logger.info(
+            "Kolibri instance '%s' joined the network; device info: %s"


joined zeroconf network → joined the network, same for update and leave. Operator-visible; note it in the PR body.

Noted in the PR body, under the summary.

rtibbles · 2026-08-16T07:20:58Z

+        # drop references into the finished backend so this process-lifetime
+        # singleton doesn't keep the old backend (and its buses/instance) alive
+        self.instance = None
+        self._reset_backend_callbacks()


local_names isn't cleared here, only in unregister(). Harmless while the backend unregisters first, but the two teardown paths clear different halves of a process-lifetime singleton. Clear it here too.

Cleared here too, covered by test_stop_listening_drops_local_names.

rtibbles · 2026-08-16T07:20:58Z

+        self.transport.start_listening(
+            self.on_add, self.on_update, self.on_remove, self.is_known
+        )
+        mock_browser.assert_called_once()


Asserts a browser was built, not that it's ours — nothing pins handlers=[transport._handle_service_change], and nothing asserts stop_listening() closes Zeroconf. Both underpin the _is_current guards.

Pinned: test_start_listening_attaches_browser now asserts ServiceBrowser(self.zeroconf, SERVICE_TYPE, handlers=[transport._handle_service_change]) and that the return value is what lands in browsers["bus"]. Added test_stop_listening_closes_zeroconf for the close plus the zeroconf = None the _is_current guards match against.

rtibbles

This is looking good, just one question about separation of concerns.

rtibbles · 2026-08-18T01:01:57Z

+    `KolibriInstance` objects and dispatches them to the backend callbacks
+    stored by `start_listening`.
+
+    Kept a plain class (not a `KolibriHook`) so `broadcast.py` stays importable


I am a little confused by this - if this is now only the ZeroConf implementation, why couldn't it just be implemented in the kolibri_plugin.py? Alternatively, if it is too big for that, then having a separate zeroconf module would seem sensible?

I am not against it being in here, I just mostly want to make sure that keeping it here isn't letting us be lax in the separation of concerns.

Moved it. ZeroconfNetworkDiscovery plus the zeroconf-only constants and helpers (SERVICE_RENAME_ATTEMPTS, the .local alias types, EVENT_UPDATE_LOCAL_NAMES, slugify_device_name, filter_lan_addresses, get_outgoing_interface_address) now live in utils/network/zeroconf_transport.py, with the transport tests in test/test_zeroconf_transport.py. Too big for kolibri_plugin.py at ~530 lines, and a plain class keeps importing it free of plugin registration — the registered mix-in stays in kolibri_plugin.py.

Swept the branch for the same mixing: three other candidates, none changed.

ZeroConfPlugin — only names left (UPDATE_ZEROCONF, ZEROCONF_ENABLED), no zeroconf implementation.

hooks.py — transport-neutral already.

KolibriInstance and SERVICE_TYPE/LOCAL_DOMAIN stay in broadcast.py. It is the shared model tasks.py and search.py consume, and zeroconf_id is the DynamicNetworkLocation pk, so that is real residual coupling I left rather than renamed — locations and tasks are out of scope here.

Split the zeroconf-welded KolibriBroadcast into a transport-agnostic NetworkDiscoveryBackend and a NetworkDiscoveryHook transport interface, so a platform can supply an alternative discovery transport (e.g. Android native NSD) and run discovery outside the server ProcessBus. Zeroconf becomes the default-registered transport; server behaviour is unchanged. - Define NetworkDiscoveryHook, a single-registration hook with register/update/unregister/start_listening/stop_listening. - Add NetworkDiscoveryBackend: broadcast-id lifecycle, the instance event bus, the other_instances dedup cache, and listener dispatch. It resolves its transport through the hook and is constructable/start-stoppable without a KolibriProcessBus. - Extract the zeroconf logic (Zeroconf/ServiceBrowser/ServiceInfo, interface and address monitoring, .local aliasing) into ZeroconfNetworkDiscovery in its own zeroconf_transport module, registered as the default transport via ZeroconfNetworkDiscoveryHook in kolibri/core/discovery/kolibri_plugin.py. - Move the LocalHostnameListener onto the transport's own bus. - Drive ZeroConfPlugin through NetworkDiscoveryBackend and remove KolibriBroadcast and its now-dead zeroconf-only event machinery. on_remove takes the service name (a removed service can no longer be queried) and resets the cached instance's broadcasting flag, so a peer leave then rejoin re-publishes an ADD. The transport is a process-lifetime singleton, so a browser event can outlive the backend that started it: Zeroconf.close() joins the engine and reaper but not the browser threads. Handlers drop any event whose Zeroconf is no longer the one being listened on, and the backend callbacks fall back to no-ops between sessions so a dispatch racing the stop is dropped rather than raising. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions Bot added DEV: backend Python, databases, networking, filesystem... SIZE: very large labels Jul 17, 2026

rtibblesbot marked this pull request as ready for review July 17, 2026 20:52

rtibblesbot force-pushed the issue-14996-cbedc0 branch from e0202eb to de616e4 Compare July 17, 2026 20:54

rtibbles reviewed Jul 17, 2026

View reviewed changes

rtibblesbot force-pushed the issue-14996-cbedc0 branch from 9fd6518 to 4a4903c Compare July 17, 2026 21:37

rtibblesbot force-pushed the issue-14996-cbedc0 branch from 4a4903c to 500f52e Compare July 17, 2026 22:11

rtibbles self-assigned this Jul 27, 2026