1 Commits

Include renames

Author	SHA1	Message	Date
Dario LipicarandClaude Opus 5	84564f0f6a	feat(core): turn module lifecycle into sequenced facts, and feed them to modules_state (#189) ... * feat(core): observe module lifecycle as sequenced facts, not log lines Until now load, unload and crash were spdlog::info lines and registry membership changes were silent, so every consumer polled — basecamp runs a 2s QTimer and infers module state from package-install events. ModuleStateObserver turns those into structured, sequenced transitions and hands them to a SINK. It reports only: it does not talk to any module and does not know modules_state exists. Wiring a sink that pushes to that module is the next stage. With no sink installed record() early-outs before it allocates, so a host consuming nothing pays nothing — and buffering with no consumer would be an unbounded leak in the normal case. TWO RULES, both load-bearing: 1. Never dispatch under loadMutex(). record() buffers; flush() dispatches, and every entry point declares ScopedModuleStateFlush BEFORE its lock guard so it is destroyed AFTER it. A sink doing an RPC from inside the load path while holding that lock is the shape of two failures already paid for here: the ui-host startup token deadlock, and the ~417s basecamp stall from a synchronous call to an absent module. 2. One seq counter, for deltas and snapshots alike. Consumers apply a transition only when its seq beats what they hold for that module, and keep a seq tombstone for a departed one. A second counter makes that tombstone either unreachably high (a real later delta dropped forever) or trivially low (a stale delta resurrecting a pruned module). Seams: unloaded->loading at load start, loading->loaded on success carrying instanceId and pid, loading->error on all three failure paths, loaded->stopping->unloaded on unload, and the membership edges absent->unloaded / unloaded->absent in discovery and prune. processModule() gets the discovery edge too — it is a second way a module enters the registry, and a consumer that only saw scan edges would be surprised by a `unloaded -> loading` for a module it had never heard of. Two bugs found while wiring it, both of which would have made the feed lie: * onTerminated fires for BOTH an orderly unload and a module that died, and cannot tell them apart from its arguments. Teardown now announces intent before terminate(); the callback consumes it, once, so an unload/reload/crash still reads as a crash. * terminateAll() and clear() tear down every loaded module at once, so a CLEAN HOST SHUTDOWN would have reported the entire fleet as crashed — `loaded -> error`, "module exited without being asked to", once per module. They now announce every loaded module first. 9 tests, and they are proven to bite: making record() dispatch inline (the rule-1 violation) fails RecordDoesNotDispatch and reddens the check. Full suite 194/194. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(core): feed module lifecycle to modules_state The consumer end of ModuleStateObserver. The observer produces sequenced transitions and knows nothing about any module; this turns them into calls on `modules_state`, so the state liblogos has always had stops being spdlog lines and becomes queryable and subscribable. Modelled on the capability_module push already in this file -- one long-lived "core" LogosAPI, per-module transport honoured -- with three differences, all forced by where it runs: 1. ASYNC deltas. registerRestrictionRpc is synchronous and gets away with it because it is rare and short. This runs on EVERY load, unload and crash, from the observer's flush, on whichever thread did the work. A synchronous RPC there would put a 20 s worst case on the load path. 2. CHEAP NO-OP WHEN ABSENT, checked before the client is even fetched. A synchronous dial to a module that is not there cost Basecamp ~417 s of blocked GUI thread once already. 3. THE SINK IS UNINSTALLED when modules_state unloads, so the observer goes back to buffering nothing rather than buffering into a sink that drops. SNAPSHOT ON AVAILABILITY, NOT ON LOAD. modules_state loads after other modules, so deltas alone give it a permanently short list -- which is what `partial` exists to signal. The snapshot clears it. It is armed with whenObjectAvailable() rather than fired at load, because `load-module` returns when the plugin is IN and the module PUBLISHES later -- measured elsewhere at ~390 ms on a cold start -- and whenObjectAvailable is the primitive that waits without failing fast or burning the acquire timeout on the calling thread. ONE SEQ COUNTER. Every record seq and the listing seq come from the observer's counter, listing drawn LAST so it is >= every record in it. modules_state tombstones a pruned record at the LISTING's seq, so a second counter would make that tombstone either unreachably high (a real later delta dropped forever) or trivially low (a stale delta resurrecting a pruned module). partial:false is a claim, and it is defensible: `partial` means the host's scan SKIPPED something, and discoverInstalledModules drops a module it cannot read before it ever enters the registry. Anything missing is not something the host knows and is withholding; it is something the host does not know. PROVEN END TO END against a live daemon, with NO test door open -- so the facts arriving also prove the module's structural gate admitted core as kind=host: snapshot: "Pushed module snapshot to modules_state", and list_modules then reports both modules with real paths and load timestamps, partial:false, seqs 1/2 under listing seq 3. delta: unload capability_module -> its record goes loaded(seq 2) -> unloaded(seq 5) through the ASYNC path, which is separate code from the snapshot's synchronous one and needed proving separately. refusals: 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: cut the comment walls Same pass as the module side. The prose had grown past the point where it helps. module_state_observer.h 174 -> 127 lines, 103 -> 56 comment module_manager.cpp 285 -> ~215 comment module_registry.cpp / observer.cpp / tests / README trimmed to match Kept the non-obvious why: the two rules (never dispatch under loadMutex, one seq counter for deltas and snapshots), orderly-teardown vs death, why a clean shutdown would otherwise report the fleet as crashed, and why partial:false is a defensible claim rather than an assumption. The worst offender was mechanical: the four-line "declare the flusher before the lock guard" explanation was pasted at all SEVEN call sites. It is now one line pointing at the rule in the header, where the explanation lives once. 194/194. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(core): emit loaded -> ready when a module publishes `loaded` means the host owns the process: markLoaded runs once the child is spawned and its token is written, which is strictly before the module publishes its object (~390ms cold for modules_state; 826ms measured for a module with an empty initializer). Nothing emitted that gap, so consumers had to treat "loaded" as "callable" and be wrong for the width of the window. Adds a second edge rather than moving the first. `loaded` stays an ownership fact -- total across loaders, and the thing unload and the double-spawn guard need. `ready` is the readiness fact, observed asynchronously. armReadinessWatch arms a one-shot whenObjectAvailable and returns: it never waits, so rule 1 (no dispatch under loadMutex) holds, and the callback lands with no lock held. Only armed when a sink is installed -- without a consumer each watch would hold a client and a replica for nothing, and it keeps the FakeModuleLoader tests, which publish nothing, arming nothing. The callback carries the loadEpoch it was armed under and markPublished drops it if that no longer matches, so a fast unload/reload cannot let a stale watch mark the new instance ready. Epoch rather than loadedAt: the latter is whole seconds and collides. modules_info gains `published` / `published_at`. published is null, not false, when no watch is armed -- "nobody looked" and "not ready" are different answers. Also folds capabilityModuleClient and modulesStateClient into one moduleClient(name); they were identical apart from the name. checks.tests green: 194 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-08-26 18:23:27 -03:00