test universal modules with LogosModuleContext

[dlipicar]

Summary

Adds a universal C++ test module — test_context_module_cpp — that exercises every wire-up the new LogosModuleContext mixin provides, end-to-end, and rewires the integration runner so the existing CLI-driven check rig covers it.

Concretely:

• A new module that inherits LogosModuleContext and surfaces all four kinds of context wiring as plain Q_INVOKABLE shims callable from logoscore:
  • Lifecycle hook — wasContextReady() delegates to the SDK's isContextReady() (returns true once the framework's _logosCoreSetContext_ has populated the three getters and fired onContextReady()).
  • Host-injected properties — getModulePath(), getInstanceId(), getInstancePersistencePath(), plus hasInstanceId() and persistencePathEndsWith(suffix) so the integration runner can make positive assertions against opaque host-generated values.
  • Typed cross-module calls — callBasicEcho / callBasicAddInts go through modules().test_basic_module.<method>(...), exercising the full onInit → std::make_unique<LogosModules>(api) → _logos_codegen_::maybeSetLogosModules(m_impl, ptr) → LogosModuleContext::_logosCoreSetLogosModulesPtr_ chain at runtime, plus the typed <Module> wrapper at compile time.
  • Typed event subscriptions — subscribeToBasicCppEvents() registers modules().test_basic_module_cpp.onTestEvent(...) and onMultiArgEvent(...), capturing payloads into instance state; getLastTestEventData() / getLastMultiArgEvent() read them back. Used by the run-time subscribe → trigger → read round-trip via chained logoscore -c.
• Switches test_basic_module_cpp over to the new mechanisms in the same PR: inherits LogosModuleContext, declares its events in a logos_events: block, and exposes bool-returning triggerTestEvent / triggerMultiArgEvent shims (the void-returning emit* Q_INVOKABLE pair can't be chained via logoscore -c).
• Adds a context-cpp group to tests/run_tests.sh (helper test_context_cpp always passes --persistence-path so the host provisions a per-instance dir against a tmpdir the test framework knows the prefix of) and wires test_context_module_cpp into flake.nix's module outputs, install aggregation, and modulesDir build.
• Adds two subscribe → trigger → read end-to-end cases under the context-cpp group that chain test_context_module_cpp.subscribeToBasicCppEvents() → test_basic_module_cpp.triggerTestEvent(hello) → test_context_module_cpp.getLastTestEventData() in a single logoscore invocation, asserting the typed payload round-trips.

CI failures on this PR are expected until logos-cpp-sdk#61 lands — they're caused by master's logos-cpp-sdk not yet exposing LogosModuleContext, modules(), or the logos_events: mechanism. With the SDK PR's branch overridden locally (logos-cpp-sdk, logos-module-builder, logos-plugin-qt, logos-plugin-core all pointing at the matching worktree paths), the suite passes 169/169 (no flake).

Review feedback addressed

1. "Available groups" list missing async (Copilot, tests/run_tests.sh:23)

Added async to the comment-block enumeration.

2. Misleading liveness-probe comment (Copilot, tests/run_tests.sh:495, low-confidence note)

The pre-existing comment claimed the probe "returns the int 1 via a constant" — leftover from an earlier version. Rewrote to describe what the assertion actually does:

Quick liveness probe: asserts the SDK flipped LogosModuleContext's isContextReady() flag and fired the onContextReady() hook before the first method dispatch — wasContextReady() returns true. Failure here means the framework didn't even wire the context, and the rest of the assertions are noise.

3. getInstanceId() non-empty assertion was too weak (Copilot, tests/run_tests.sh:519)

The original assertion just greps for the literal substring Result: — which would pass for any string return, including empty. Replaced with a bool-returning helper on the impl:

// test_context_module_cpp_impl.h
bool hasInstanceId() const;        // returns !instanceId().empty()

# tests/run_tests.sh
test_context_cpp "hasInstanceId() == true"  "Result: true"  \
    "test_context_module_cpp.hasInstanceId()"

Result: true only appears when the underlying string is non-empty, so the assertion actually distinguishes "host populated the property" from "default-empty fallback".

4. Stale Qt/QString comments on universal-typed code (Copilot, test_context_module_cpp_impl.h:73 and test_context_module_cpp_impl.cpp:66)

The header comment claimed the call site lives in the .cpp because "the typed wrapper is generated against Qt types (QString in / out)" and the cpp comment claimed "QString↔std::string bridge ... lives entirely in this .cpp." Both wrong — the module is interface: "universal", so the dep wrappers are std-typed and no bridge is needed. Rewrote both to say so:

// callBasicEcho (in the .h, summarised):
The actual call site lives in the .cpp because this module is interface: "universal" — the codegen emitted the dep wrapper with std-typed signatures, and modules() requires the generated logos_sdk.h to be complete in the TU. The .cpp includes that header; the impl .h stays free of it so the codegen parser sees only the C++ surface it expects.

5. Wrong capture-mode comment on subscription state (Copilot, test_context_module_cpp_impl.h:111)

Comment said "callbacks are registered with & capture" but the actual lambdas in subscribeToBasicCppEvents() capture [this]. Corrected:

The two callbacks capture [this] and write through to these members.

Test plan

• nix build .#checks.aarch64-darwin.tests from repos/logos-test-modules with the four overrides (logos-cpp-sdk, logos-module-builder, logos-plugin-qt, logos-plugin-core all overridden to the matching worktree branches) — 169 passed, 0 failed, 25 skipped of 169 run.
• Context-cpp group covers all four kinds of wiring: lifecycle (wasContextReady), getters (3× context properties + hasInstanceId), cross-module calls (callBasicEcho, callBasicAddInts), and typed event subscriptions (subscribeToBasicCppEvents + testEvent/multiArgEvent round-trips via chained -c).
• CI on PR head will stay red until logos-cpp-sdk#61 merges — master's SDK doesn't yet expose the symbols this module uses.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Adds a new universal C++ test module to validate LogosModuleContext lifecycle wiring (context properties + modules() + typed events), and extends the integration test runner / Nix flake outputs to build and execute it as part of the standard test suite.

Changes:

• Introduces test_context_module_cpp (universal module) that reads LogosModuleContext properties, makes cross-module calls via modules(), and subscribes to typed events from test_basic_module_cpp.
• Extends tests/run_tests.sh with a new context-cpp test group that runs the new module with --persistence-path and performs end-to-end assertions (including chained -c invocations).
• Updates test_basic_module_cpp to inherit LogosModuleContext and declare typed events via logos_events:, plus wires the new module into flake.nix outputs and the combined modulesDir.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/run_tests.sh	Adds context-cpp group and helper to run the new module with --persistence-path.
test-context-module-cpp/src/test_context_module_cpp_impl.h	Declares the new module’s context accessors, cross-module calls, and event subscription methods.
test-context-module-cpp/src/test_context_module_cpp_impl.cpp	Implements the new module’s LogosModuleContext introspection, cross-module calls, and event subscriptions.
test-context-module-cpp/metadata.json	Defines the new universal module and its dependencies (test_basic_module, test_basic_module_cpp).
test-context-module-cpp/CMakeLists.txt	Adds build configuration for the new module via logos_module(...).
test-basic-module-cpp/src/test_basic_module_cpp_impl.h	Switches to LogosModuleContext inheritance and declares typed events under logos_events:.
test-basic-module-cpp/src/test_basic_module_cpp_impl.cpp	Updates event emission to call the typed event methods and adds bool-returning trigger helpers for CLI chaining.
flake.nix	Adds test_context_module_cpp to flake module outputs, install aggregation, and test modulesDir construction.

Comments suppressed due to low confidence (1)

tests/run_tests.sh:495

• This comment says the liveness probe “returns the int 1 via a constant”, but the probe actually calls wasContextReady() and asserts Result: true. Update the comment to reflect what’s being tested to avoid misleading future maintainers.

# Quick liveness probe: returns the int 1 via a constant — confirms
# the module loads and the codegen-emitted dispatch wires up before
# we start asserting context state. Failure here means the rest of
# the assertions are noise — the framework didn't even start.
echo ""
echo "  -- Lifecycle hook fired --"
test_context_cpp "wasContextReady()"    "Result: true"  "test_context_module_cpp.wasContextReady()"

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.