diff options
| author | Christophe Besson <cbesson@gmail.com> | 2026-09-06 21:43:00 +0200 |
|---|---|---|
| committer | Christophe Besson <cbesson@gmail.com> | 2026-09-06 21:43:00 +0200 |
| commit | 232fd2a8d15f63b6bf6ad26286dd9836d6819668 (patch) | |
| tree | 11963304acb5cee3bb500d8c2cc166f3953d2752 /packages/meshbay-node/src/meshbay_node | |
| parent | 4c4e9ba7a17e058dc12cb10171743329201dd7e6 (diff) | |
| download | meshbay-232fd2a8d15f63b6bf6ad26286dd9836d6819668.tar.gz | |
fix(node): the MNP root path never reloaded, and my first repair made it worse
The previous commit was the wrong fix. `add_root` did leave the running node
unchanged, but editing the live `RootSet` in place — which is what I did — is
wrong in the other direction.
`DirectoryIndexer.retarget` decides what to scan by diffing the names it
already holds against the ones it is handed, and `_retarget_indexer` hands it
`groups_ctx[gid]["roots"]`: the very object the op had edited. So the new root
sat on both sides of the comparison, nothing was scanned, and the directory
would have appeared in the table permanently empty. `_reload_config_inner`
diffs the same way and would have concluded nothing changed. `remove_root` had
the same shape and would have kept serving a removed directory's files.
The real defect is that two front doors did different things. `ui/app.py` has
always fired the daemon's `reload_fn` after these ops, which re-reads node.toml
and builds a *fresh* set; the MNP path retargeted a stale object instead. That
asymmetry is exactly what `ops.py` exists to prevent, and it is why the bug
survived until an operator added a directory from a browser — the loopback path
worked all along.
So: the ops leave the live set alone, `_retarget_indexer` asks the daemon to
reload, and `update_root` keeps editing in place because flags change no files
and the synchronous upload handler reads that object on the next request.
The tests now check the files rather than `describe()`, which proves nothing
about whether anything was scanned. One of them demonstrates the failure mode
instead of describing it, so the rule is checkable and will say so if
`retarget` ever changes. Two more cover the seam itself — that the MNP path
reloads, and that a context with no daemon still retargets.
Diagnosed by reading the running node's journal rather than the source: the
first add logged "Reloading config" and a rescan, the two later ones logged
neither. I should have looked there before the first attempt.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011pvMdvLBG92jyhvD5pD6us
Diffstat (limited to 'packages/meshbay-node/src/meshbay_node')
| -rw-r--r-- | packages/meshbay-node/src/meshbay_node/ops.py | 50 | ||||
| -rw-r--r-- | packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py | 28 |
2 files changed, 45 insertions, 33 deletions
diff --git a/packages/meshbay-node/src/meshbay_node/ops.py b/packages/meshbay-node/src/meshbay_node/ops.py index 10f6a8f..5b10452 100644 --- a/packages/meshbay-node/src/meshbay_node/ops.py +++ b/packages/meshbay-node/src/meshbay_node/ops.py @@ -682,30 +682,21 @@ async def add_root(state: dict, group_id: str, path: str, *, writable=added.writable, removable=added.removable, direct=added.direct)) - # The *live* set, not only the config — the same thing `remove_root` and - # `update_root` do, and the one this was missing. + # Deliberately *not* mutating the live RootSet in place. # - # `_retarget_indexer` (the MNP path) re-points the indexer at - # `groups_ctx[gid]["roots"]`, so leaving that object untouched retargeted - # it at exactly what it already had: node.toml gained the directory, the - # index went on reporting the old set, and the next `index_sync` overwrote - # whatever the ack had just told the client. The root was there, invisible, - # until a restart — and adding it again was refused as a duplicate of - # itself, which is the only reason anyone found out. + # `DirectoryIndexer.retarget` decides what to scan by diffing the names it + # already has against the ones it is given — so handing it the same object, + # edited, means the new root is in both sides of the comparison and is + # never scanned. It would appear in the table and stay permanently empty. + # `_reload_config_inner` diffs the same way and would likewise conclude + # nothing changed. The caller reloads instead, which builds a fresh set + # from the file this just wrote. # - # Safe to append rather than rebuild: `RootSet.build(specs)` above already - # validated the whole set, this root included, for name collisions and - # nesting. - live_roots: RootSet | None = state.get("groups_ctx", {}).get( - group_id, {}).get("roots") - if live_roots is not None and not any( - r.folded == added.folded for r in live_roots.roots): - live_roots.roots.append(added) - + # `built` is that set, computed here only to validate and to answer with; + # what the node serves comes from the reload. log.info("Root added: %s → group %s", added.name, group_id[:8]) return {"status": "added", "name": added.name, "path": str(added.path), - "group_id": group_id, - "roots": (live_roots or built).describe()} + "group_id": group_id, "roots": built.describe()} async def remove_root(state: dict, group_id: str, root_name: str) -> dict: @@ -741,20 +732,15 @@ async def remove_root(state: dict, group_id: str, root_name: str) -> dict: cfg.roots.pop(match_idx) - # Update the live RootSet so GET /api/groups returns correct data - # immediately, without waiting for the async reload. - live_roots = state.get("groups_ctx", {}).get( - group_id, {}).get("roots") - if live_roots: - live_roots.roots = [ - r for r in live_roots.roots if fold(r.name) != target] - - # Built from config when there is no live set, never returned empty: an + # Not mutating the live set here either — see `add_root`. Dropping the + # root from it would leave `retarget` unable to tell that its entries + # should go, so the removed directory's files would stay in the index. + # + # Built from the config this just edited, and never returned empty: an # empty list is a *valid answer* meaning "this group has no directories", - # and the client cannot tell it from "the node could not say". It would + # which the client cannot tell from "the node could not say" — it would # blank the operator's table on an op that succeeded. - result_roots = (live_roots.describe() if live_roots - else RootSet.build([asdict(r) for r in cfg.roots]).describe()) + result_roots = RootSet.build([asdict(r) for r in cfg.roots]).describe() log.info("Root removed: %s from group %s", root_name, group_id[:8]) return {"status": "removed", "name": root_name, "group_id": group_id, diff --git a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py index c4d053e..d341d8c 100644 --- a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py +++ b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py @@ -2773,10 +2773,36 @@ class WebRTCPeerSession: return await fn(state, *args, **kwargs) async def _retarget_indexer(self, group_id: str) -> None: - """Tell the indexer to rescan after roots changed.""" + """ + Pick up a root that was just added to or removed from node.toml. + + Through the daemon's own reload, which is what the loopback API has + always done after the same operations (`ui/app.py`). This used to + re-point the indexer at `groups_ctx[gid]["roots"]` instead — the very + object the op had just edited — so `retarget` diffed a set against + itself, found no new names, scanned nothing, and dropped nothing. A + directory added over MNP reached node.toml and was invisible until a + restart; one removed kept serving its files. + + Two front doors doing different things is the shape `ops.py` exists to + prevent, and this was it: the loopback path worked and the MNP path did + not, which is why it survived until the operator added a directory from + a browser. + + Not awaited: a reload rescans, and a new library is minutes. The ack + the caller sends carries the set the node is moving to, and the + `index_sync` that follows the scan carries what it found. + """ state = self._ctx.get("daemon_state") if not state: return + reload_fn = state.get("reload_fn") + if reload_fn: + self._spawn(reload_fn()) + return + # No daemon to ask — a test harness, or a context assembled by hand. + # Retarget directly, which is correct as long as the caller did not + # edit the live set in place. indexer = state.get("indexers", {}).get(group_id) roots = state.get("groups_ctx", {}).get(group_id, {}).get("roots") if indexer and roots: |