Repository navigation
Node adoption from the UI + missing-node alert + project_status loaded/uuid (869ebkkme, 869dr2kzh) - #32
Merged
Conversation
…lling
The UI, the editor relay and cuems-nodeconf all implement adopt/un-adopt.
The engine hop never did: handle_editor_command's dispatch table has no
'nodelist_modify' key, so every click from the settings panel reached
`raise ValueError("Command nodelist_modify not recognized")`. The wiring was
written once (4e5680d) but survives only under refs/tags/archive/
feat-node_adoption, an ancestor of no branch.
Re-landed, with the three things that version got wrong:
* modify_action now reaches nodeconf. Every command_dict entry is invoked as
(value, context) — a two-argument contract with nowhere to put it — so a
table-registered handler would send modify_action=None and nodeconf would
answer "Invalid modify_action: None" to every single click. The message is
bound before the table and dispatched directly.
* A None reply is handled. Communicator.send_request swallows every exception
and returns None, which is what an absent /tmp/nodeconf.ipc looks like — and
nodeconf ships disabled on most of the fleet. The old code called .get() on
it and raised AttributeError.
* Failures reply exactly once, and never by raising. editor_command_callback's
except clause also replies, so a raise reaches the operator as "Engine
reports error: Command <class 'RuntimeError'>: ..." and a handler that
replies *and* raises puts two replies on one NNG Rep context.
Adoption is now refused while a project is running AND while one is merely
loaded: _resolve_cluster_state computes _required_nodes once, at load_project,
so a REMOVE accepted mid-load would leave the GO gate waiting on a node the
operator just removed. Refusing is the option that never stops anything.
On success the engine re-reads network_map.xml and re-registers the per-node
OSC routes, because its own copy was loaded once at startup — without this the
adopted node stays invisible to the GO gate until a restart, i.e. the button
would report success and change nothing observable. If that reload fails the
operator is told it is a partial success, not given a green toast over a stale
topology.
15 tests, and the whole test_controller_commands/gating/nodehub set still green.
Refs: 869ebkkme
Plan: cuems-RELATIONS/Plans/2026-09-04-nodelist-modify-adoption-path.md
nodelist_modify is deliberately NOT in command_dict — every entry there is called as (value, context), so a table-registered handler could never receive modify_action and nodeconf would reject every click. Records that, plus: a missing nodeconf socket returns None rather than raising (and nodeconf is disabled on most of the fleet, so that is the common path); failures must reply exactly once and never by raising, or the operator reads "Engine reports error: Command <class 'RuntimeError'>: ..."; and adoption is refused while a project is loaded because _required_nodes is computed once, at load. Also documents the targeted network_map re-read after a successful change — including that get_nodes_by_adoption mutates in place and survives one pass, so the OSC re-registration is only valid on the freshly reloaded dict — and qualifies the "restart both daemons" note, which now has exactly one exception. Refs: 869ebkkme
…lear it
Adversarial review of Part A found that nothing asserted the one line the plan
called out by name — handle_editor_command's set_editor_request("") after a
successful confirm, which the archived version omitted. Deleting it would have
failed no test. Now two tests cover it, on the success and the refusal path.
_nodelist_refuse also left _editor_request_uuid set, unlike the generic error
path which captures it, clears it, and replies with it explicitly. Harmless
today because editor_command_callback overwrites it on every call, but it was
an unexplained deviation from the one-reply-and-clean-state discipline the rest
of this handler follows.
73 passed across nodelist_modify + controller_commands + controller_gating.
Refs: 869ebkkme
Found on the rig, and it contradicts what the plan assumed. With /tmp/nodeconf.ipc absent, request_to_nodeconf does NOT return None quickly — it burns the engine's full 15 s IPC timeout and then raises TimeoutError. Because editor commands are serialized by the engine's single listener, a second click queues behind the first, so the operator's second attempt can hit the editor's own 25 s timeout and read "Engine did not respond" instead of a reason. And nodeconf ships disabled on most of the fleet, so this is the common path, not the rare one. Now the socket is checked first: the refusal is instant (measured 0.00 s on the rig, against ~15 s before) and says what to do — "systemctl enable --now cuems-nodeconf". The IPC call also carries an explicit 5 s timeout; adopt/un-adopt is a local XML rewrite and has no business taking the engine-wide 15 s. The None-return handling stays: send_request still swallows other failures into it. Rig, on the FormitGo N97 controller: adopt answered in 0.03 s with nodeconf up, refusal in 0.00 s with it down. 48 tests green here (nodelist_modify + controller_commands), 19 in this file. Refs: 869ebkkme
The engine already knows who is answering — _probe_cluster_liveness is what the GO gate trusts — but that view never left the process, so the UI had no way to tell "known to nodeconf but absent" from "dead right now". Adds a read-only cluster_status editor command returning alive / adopted / controller / age_s. age_s is computed here rather than handing out a monotonic timestamp the browser cannot interpret. Repeat calls inside 2 s are served from cache: each probe pings every adopted node and the settings panel polls while it is open. The answer is always a populated dict — the dispatch path confirms only on a truthy result, so an empty one would leave the editor waiting out its 25 s timeout. Also writes down, at the probe itself, the invariant this makes load-bearing: the pong bookkeeping is shared instance state with no correlation id, so two concurrent probes would eat each other's replies. Nothing enforces that — it holds only because the engine's single editor listener awaits one handler before accepting the next. Until now the only caller was project load; the UI can reach it now, so a future "process editor commands concurrently" change must add a lock or a correlation id first. Rig (FormitGo N97): answered in 0.01 s; on a single-box controller alive == adopted == [controller] via the no-remote-nodes early return, with no ping broadcast; three rapid calls produced age_s 0.0/0.001/0.002 and exactly one real probe in the journal. 25 tests in this file. Refs: 869ebkkme
Un-adopt a node — or let it die — then load a project whose cues use its
outputs: the load answers OK, the GO gate turns green, and those cues
never fire. _resolve_cluster_state already knew, and only told the
journal.
Two defects, one of them a lie. `required` is (adopted & alive & project)
| {controller}, so a project node that did not pong is EXCLUDED from the
gate — yet the log line for exactly that case said "GO blocked". Anyone
debugging a mute show from the journal was sent the wrong way.
The operator decision is warn loudly, never block GO, so `required` is
untouched here and a test pins it against the original formula in every
shape. What changes is that the diagnosis leaves the process:
* missing — project nodes not in the cluster at all
* unreachable — adopted and used by the project, but silent
Three surfaces, because none alone reaches the operator: pushed at load
as /engine/status/cluster_warning (JSON, since OSC carries no lists),
replayed in _on_ws_client_connect (with boot auto-load the browser is
opened after the load), and readable in get_cluster_status → the
editor's node_status, read fresh outside the 2 s clamp so a poll landing
just after a load cannot serve the pre-transition answer.
Always sent, empty lists included: a clean load has to actively erase the
previous warning. Cleared on unload and at the top of load_project, so an
early failure does not leave the last project's warning on screen — but
NOT from _clear_playback_state, which also runs on STOP, where the
project is still loaded and its missing nodes still missing.
load_id increments per load so a UI can tell an operator's retry
(identical lists, new id) from a reconnect replay (same id).
The controller is excluded from both lists: it is in `required`
unconditionally and is not always flagged adopted in network_map.xml, so
including it raised a false alarm on a healthy load — caught by the test
written for it. A false alarm teaches operators to ignore the real one.
Log lines now carry role_id/alias where the map has them; wire payloads
stay UUID-only, since names are mutable projections the browser already
resolves.
Tests: +23 (test_cluster_warning.py). test_project_go.py cannot run on
casas — it wedges in snd_seq_open, a pre-existing ALSA collision on that
box, unrelated to the GO path this commit does not touch.
Plan: cuems-RELATIONS/Plans/2026-09-04-missing-node-alert.md
Loaded-but-stopped collapsed into status "none", so a client could not tell a project that is loaded and waiting from no project at all — while /engine/status/load went on naming it all along. That contradiction is the one power-bridge's field notes recorded as "engine says project_status=none but keeps broadcasting a stale loaded". Three states now: running, loaded, none. project_uuid is populated in the first two, and the invariant is one-way — a status other than "none" always carries a uuid, so running-with-no-script degrades to none rather than offering a running the client cannot act on. project_unix_name rides along so a consumer needing the uuid never forces the broadcast to carry one; that is how 869dr2k2h is closed, additively. The TODO asking for the opposite is removed and replaced by the reasoning: the frontend prints that string in the transport bar and matches it against project.unix_name, so a uuid there shows a raw uuid and breaks the match. The load state is read from two attributes a concurrent load can be mutating, so they are ANDed: a torn read falls to "none", never to a "loaded" naming a project the engine does not hold. Second defect, same family: load_project dropped self.script before validating the new project but never dropped `load`, so a failed load left the engine holding nothing while still broadcasting the previous project's name. The transport bar showed it and power-bridge believed it, accepting /setnextcue and /gocue against a project that was gone. The clear goes next to _clear_load_diagnosis(), which is there for the same reason, so the two signals die together. Deliberate cost: during a successful load the bar reads "—" until the new name arrives, instead of the stale old one. Tests: the one asserting the old behaviour is rewritten, plus twelve covering the four windows where a single signal lies, the uuid invariant, Uuid→str coercion, a script without unix_name, and the running guard leaving `load` alone. 869dr2kzh, 869dr2k2h
Only conflict: the SPDX header form of CLAUDE.md (kept rc_1's). load_project now clears 'load' at entry (project-status) and bursts cue names after the final 'load' (869fedahu) - the clients clear their name table on every change of 'load', so both orders hold. Same tree as the integ/rc6-3-node-adoption build (0.1.0rc6-3+nodeadopt2) that test and test2 run since 2026-10-09, minus its version suffix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lands the node-adoption engine side, written 2026-09-04 and run on the taller rigs ever since:
/engine/status/cluster_warningtells the operator when a project's nodes are missing or unreachable — it warns and never blocks GO (requireduntouched, pinned by a parametrised test).loadand the diagnosis instead of leaving the previous project's name on the wire.Merge of
rc_1into the branch: onlyCLAUDE.mdconflicted (SPDX header form).load_projectclearsloadat entry, and the 869fedahu cue-name burst still follows the finalload; clients clear their name table on every change ofload, so both orders hold. Plus ablackpass on the branch's own files (CI lint).Same tree as the
0.1.0rc6-3+nodeadopt2test build that test and test2 have run since 2026-10-09 (load arms all nodes, GO plays). Full suite on test2: 1292 passed (idle rig, scratch copy). black / isort / flake8 clean.Frontend side already on
dev; editor PR and nodeconf PR land alongside.