Node adoption from the UI + missing-node alert + project_status loaded/uuid (869ebkkme, 869dr2kzh)

CLAUDE.md

@@ -112,7 +112,22 @@ Engine → VideoComposer and engine → DmxPlayer are inter-component protocols
 
 ## Field notes / gotchas
 
-- **Editing `network_map.xml` or `default_mappings.xml` requires restarting BOTH `cuems-controller-engine` AND `cuems-editor`** (plus `cuems-node-engine` if active) — both daemons load topology **once at startup** via `ConfigManager(load_all=True)` and hold it in memory; there is no per-project-load reload. Edit → `xmllint --noout --schema /etc/cuems/<x>.xsd <file>` → confirm no project loaded → restart. Verify: editor logs `"number_of_nodes": N`; engine logs correct `Controller IP` + adopted set.
+- **Node adoption from the UI (`nodelist_modify`) is a three-hop chain, and the engine hop is new.** browser → editor (WS `:9092`) → engine (`/tmp/editor.ipc`) → nodeconf (`/tmp/nodeconf.ipc`). The engine end lives in `ControllerEngine.nodelist_modify` and is **deliberately not in `command_dict`**: every entry there is invoked as `(value, context)`, a two-argument contract with nowhere to put `modify_action`, so a table-registered handler would send `modify_action=None` and nodeconf would answer `Invalid modify_action: None` to *every* click. The message is bound before the table and dispatched directly. Three more things that bite:
+  - **A missing nodeconf is the common case** (it ships disabled on most of the fleet), and it is *not* cheap to discover: measured on the rig, with `/tmp/nodeconf.ipc` absent `request_to_nodeconf` does **not** return quickly — it burns the full 15 s IPC timeout and then raises `TimeoutError`. Editor commands are serialized by the engine's single listener, so a second click queues behind the first and can blow the editor's own 25 s timeout, turning a clear refusal into *"Engine did not respond"*. Hence the `os.path.exists(NODECONF_IPC_PATH)` pre-check (instant, actionable refusal — measured 0.00 s) and the explicit 5 s `timeout=` on the call. The `None` return is still handled: `Communicator.send_request` swallows exceptions and returns it on other failures.
+  - Failures must reply **exactly once and never by raising**: `editor_command_callback`'s `except` 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. Use `_nodelist_refuse`.
+  - It is **refused while a project is running or merely loaded**. `_resolve_cluster_state` computes `_required_nodes` once, at `load_project`; a REMOVE accepted mid-load would leave the GO gate waiting on a node the operator just removed.
+- **A successful adoption re-reads `network_map.xml` in place** (`_reload_network_map` → `cm.load_network_map()` + `_register_node_osc_handlers()`), so the new node reaches the GO gate without an engine restart. ⚠ Order is load-bearing: `get_nodes_by_adoption()` mutates `online`/`adopted` from `str` to `bool` **in place** and survives only one pass, so the re-registration is valid only on the freshly reloaded dict — never call it as a standalone "shortcut" refresh. If the reload fails the operator is told it is a **partial success** (map changed, this process stale) instead of getting an OK.
+- **A project node that is missing or silent WARNS — it never blocks GO.** `_resolve_cluster_state` computes `required = (adopted & alive & project) | {controller}`, so a project node that is not adopted, or adopted but did not pong, is simply **excluded** from the arm gate: the load succeeds, GO is enabled, and that node's cues never fire. That is a deliberate operator decision (a mute cue beats a dead show), and the diagnosis is published rather than left in the journal:
+  - `missing` = project nodes not in the map at all; `unreachable` = adopted, used, did not answer. The controller's own UUID is excluded from both — it is in `required` unconditionally, and it is not always flagged `adopted` in `network_map.xml`, so including it would raise a false alarm on a healthy load.
+  - Three surfaces, all carrying the same two lists: pushed at load as `/engine/status/cluster_warning` (a **JSON string**, since OSC cannot carry lists), replayed to every new WebSocket client in `_on_ws_client_connect` (with boot auto-load the browser is normally opened *after* the load), and readable on demand in `get_cluster_status` → the editor's `node_status`.
+  - **Always sent, empty lists included** — a clean load has to actively erase the previous warning. Cleared on `unload_project` and at the top of `load_project`, so a load that fails early does not leave the previous project's warning on screen. Not cleared by STOP: a stopped project is still loaded, and its missing nodes are still missing.
+  - `load_id` increments per load so a UI can tell an operator's retry (new id, identical lists) from a reconnect replay (same id).
+  - ⚠ It is only as good as the project's output mapping: `_collect_project_nodes` derives node UUIDs from `output_name[:36]`, so a project whose outputs are unmapped or malformed yields an empty `project` set and the alert says nothing is wrong.
+- **`project_status` has three states, and `loaded` is not `armed`.** `get_project_status` answers `running` (loaded and playing), `loaded` (loaded, not playing) or `none`, with `project_uuid` **and** `project_unix_name` populated in the first two. Until 869dr2kzh it collapsed loaded-but-stopped into `none`, so a stopped project was indistinguishable from no project — while `/engine/status/load` went on naming it, which is the contradiction `cuems-power-bridge`'s field notes recorded as "engine says `project_status=none` but keeps broadcasting a stale loaded". Two traps to keep in mind:
+  - **`loaded` says nothing about readiness.** The GO gate waits on `armed` (`/engine/status/armed`); a project whose nodes never answer sits in `loaded` for the whole 120 s arm watchdog. `armed` is deliberately *not* mirrored into this reply — one pollable source per fact.
+  - **The load state is read from two attributes** (`self.script` and the `load` status) that a concurrent load can be mutating. They are ANDed, so a torn read falls to `none`, never to a `loaded` naming a project the engine does not hold.
+- **`/engine/status/load` broadcasts the `unix_name`, on purpose — do not "fix" it to a uuid.** The frontend prints this string in the transport bar and matches it against `project.unix_name`, so a uuid there shows the operator a raw uuid and breaks the match. Consumers needing the uuid ask `project_status`, which returns both (that is how 869dr2k2h was closed — additively, not by converting the broadcast). ⚠️ The status is cleared at the **top** of `load_project`, right after `reset_script()`, so it dies with `self.script`: before that, a failed load left the engine holding no project while still broadcasting the previous project's name — the transport bar showed it, and power-bridge's `project_loaded()` believed it, accepting `/setnextcue` and `/gocue` against a project that was gone. The visible cost, accepted: during a successful load the bar reads `—` until the new name arrives, instead of the stale old one.
+- **Editing `network_map.xml` or `default_mappings.xml` requires restarting BOTH `cuems-controller-engine` AND `cuems-editor`** (plus `cuems-node-engine` if active) — both daemons load topology **once at startup** via `ConfigManager(load_all=True)` and hold it in memory; there is no per-project-load reload (the one exception is the targeted re-read after `nodelist_modify`, above). Edit → `xmllint --noout --schema /etc/cuems/<x>.xsd <file>` → confirm no project loaded → restart. Verify: editor logs `"number_of_nodes": N`; engine logs correct `Controller IP` + adopted set.
 - **`cuems-node-engine` has two non-obvious hard startup blockers** (both hit on formitgo 10.16.1.111):
   1. `Requires=jack-alsa-bridges.service`, which runs `zita-j2a -d hw:PCH,0`. If that ALSA device doesn't exist (onboard codec is HDMI-only, or a USB DAC is the real card) zita fails → dependency failure → node-engine never starts. Fix: repoint `-d` to the real card (`/proc/asound/cards`; USB DAC often `hw:HID,0`), `daemon-reload` + `reset-failed` + restart.
   2. `set_video_outputs` reads `/run/cuems/display.conf` and dies with `DisplayConfNotFoundError` if there are no `[output:*]` sections with a valid `canvas_region`. A hand-authored override that only sets `resolution_policy=native` (no sections) is tolerated by videocomposer but **breaks node-engine** — keep the `[output:*]` sections too.