feat: Unified module storage, polyglot dispatch, app-layer services, WASM hook registration

bindings/node/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "amplifier-core-node"
-version = "1.0.10"
+version = "1.1.1"
 edition = "2021"
 description = "Napi-RS bridge for amplifier-core Rust kernel"
 license = "MIT"

bindings/node/__tests__/coordinator.test.ts

@@ -30,21 +30,25 @@ describe('JsCoordinator', () => {
     expect(result).toBeNull()
   })
 
-  // createHookRegistry() creates a NEW detached instance each call — this is the
-  // known limitation documented by the rename from `.hooks` getter. Use a
-  // shared JsHookRegistry if you need persistent hook registration.
-  it('createHookRegistry() returns a JsHookRegistry with listHandlers', () => {
+  // hooks is now a getter that returns the coordinator's real Arc<HookRegistry>.
+  it('hooks getter returns a JsHookRegistry with listHandlers', () => {
     const coord = new JsCoordinator(emptyConfig)
-    const hooks = coord.createHookRegistry()
+    const hooks = coord.hooks
     expect(hooks).toBeDefined()
     expect(typeof hooks.listHandlers).toBe('function')
   })
 
-  it('createHookRegistry creates a new instance each call (pins detached behavior)', () => {
+  it('hooks getter shares state — handlers registered on one call are visible on next', () => {
     const coord = new JsCoordinator(emptyConfig)
-    const h1 = coord.createHookRegistry()
-    const h2 = coord.createHookRegistry()
-    expect(h1).not.toBe(h2)
+    const h1 = coord.hooks
+    // Register a handler on h1
+    h1.register('test:event', (_event: string, _data: string) => {
+      return JSON.stringify({ action: 'Continue' })
+    }, 0, 'test-handler')
+    // h2 should see the same handler
+    const h2 = coord.hooks
+    const handlers = h2.listHandlers()
+    expect(handlers['test:event']).toContain('test-handler')
   })
 
   it('provides access to cancellation subsystem (coord.cancellation.isCancelled === false)', () => {

bindings/node/__tests__/integration.test.ts

@@ -18,8 +18,8 @@ describe('Full session lifecycle', () => {
     expect(session.sessionId).toBeTruthy()
     expect(session.isInitialized).toBe(false)
 
-    // Access coordinator (createCoordinator returns a new instance from cached config)
-    const coord = session.createCoordinator()
+    // Access coordinator (getter returns session's real Arc<Coordinator>)
+    const coord = session.coordinator
     expect(coord).toBeDefined()
 
     // Register capability and verify roundtrip

bindings/node/__tests__/session.test.ts

@@ -34,22 +34,26 @@ describe('JsAmplifierSession', () => {
     expect(session.status).toBe('running')
   })
 
-  // createCoordinator() creates a NEW Coordinator from cached config each call —
-  // this is the known limitation documented by the rename from `.coordinator` getter.
-  it('createCoordinator() returns a coordinator built from session config', () => {
+  // coordinator is now a getter that returns the session's real Arc<Coordinator>.
+  it('coordinator getter returns the session coordinator', () => {
     const session = new JsAmplifierSession(validConfig)
-    const coord = session.createCoordinator()
+    const coord = session.coordinator
     expect(coord).toBeDefined()
     // Verify coordinator was constructed from the session's config, not a default
     const coordConfig = JSON.parse(coord.config)
     expect(coordConfig).toHaveProperty('session')
   })
 
-  it('createCoordinator creates a new instance each call (pins detached behavior)', () => {
+  it('coordinator getter returns the same instance on repeated calls', () => {
     const session = new JsAmplifierSession(validConfig)
-    const c1 = session.createCoordinator()
-    const c2 = session.createCoordinator()
-    expect(c1).not.toBe(c2)
+    const c1 = session.coordinator
+    const c2 = session.coordinator
+    // Both should wrap the same underlying Arc<Coordinator>.
+    // We verify by registering a capability on one and reading it from the other.
+    c1.registerCapability('test-cap', JSON.stringify(true))
+    const result = c2.getCapability('test-cap')
+    expect(result).not.toBeNull()
+    expect(JSON.parse(result as string)).toBe(true)
   })
 
   it('setInitialized marks session as initialized', () => {

bindings/node/index.d.ts

@@ -162,22 +162,13 @@ export declare class JsCoordinator {
   registerCapability(name: string, valueJson: string): void
   getCapability(name: string): string | null
   /**
-   * Creates a new **detached** (empty) JsHookRegistry.
+   * The coordinator's hook registry — shared via `Arc`, not copied.
    *
-   * ⚠吅  **Each call returns a brand-new, empty registry** — hooks registered
-   * on one instance are invisible to the next. This is a known limitation:
-   * `Coordinator` owns its `HookRegistry` by value, not behind `Arc`, so
-   * the binding cannot share state across calls.
-   *
-   * The method name (`createHookRegistry`) intentionally signals "creates new
-   * instance" — a getter property would imply referential stability in JS.
-   *
-   * **Workaround:** create a `JsHookRegistry` directly and hold a reference.
-   *
-   * Future TODO #1: restructure the kernel to hold `Arc<HookRegistry>` inside
-   * `Coordinator` so this method can share the same registry instance.
+   * Returns a `JsHookRegistry` wrapping the coordinator's real
+   * `Arc<HookRegistry>` obtained via `hooks_shared()`. Hooks registered
+   * on the returned instance are visible to the Coordinator and vice versa.
    */
-  createHookRegistry(): JsHookRegistry
+  get hooks(): JsHookRegistry
   get cancellation(): JsCancellationToken
   get config(): string
   resetTurn(): void
@@ -190,9 +181,9 @@ export declare class JsCoordinator {
  * Lifecycle: `new AmplifierSession(config) → initialize() → execute(prompt) → cleanup()`.
  * Wires together Coordinator, HookRegistry, and CancellationToken.
  *
- * Known limitation: `coordinator` getter creates a separate Coordinator instance
- * because the kernel Session owns its Coordinator by value, not behind Arc.
- * Sharing requires restructuring the Rust kernel — tracked as Future TODO #1.
+ * The `coordinator` getter returns the session's real `Arc<Coordinator>`,
+ * and `coordinator.hooks` returns the real `Arc<HookRegistry>` — both
+ * shared, not copied.
  */
 export declare class JsAmplifierSession {
   constructor(configJson: string, sessionId?: string | undefined | null, parentId?: string | undefined | null)
@@ -212,23 +203,15 @@ export declare class JsAmplifierSession {
    */
   get status(): string
   /**
-   * Creates a new **fresh** JsCoordinator from this session's cached config.
-   *
-   * ⚠吅  **Each call allocates a new Coordinator** — capabilities registered on
-   * one instance are invisible to the next. This is a known limitation:
-   * `Session` owns its `Coordinator` by value, not behind `Arc`, so the
-   * binding cannot expose the session's live coordinator.
-   *
-   * The method name (`createCoordinator`) intentionally signals "creates new
-   * instance" — a getter property would imply referential stability in JS.
+   * The session's coordinator — shared via `Arc`, not copied.
    *
-   * **Workaround:** call `createCoordinator()` once, hold the returned instance,
-   * and register capabilities on it before passing it to other APIs.
+   * Returns a `JsCoordinator` wrapping the session's real `Arc<Coordinator>`.
+   * Repeated calls return the same underlying coordinator instance.
    *
-   * Future TODO #1: restructure the kernel to hold `Arc<Coordinator>` inside
-   * `Session` so this method can return a handle to the session's actual coordinator.
+   * Takes `&mut self` because the first call caches the coordinator internally.
+   * This is safe because NAPI JS objects are single-threaded — no concurrent access.
    */
-  createCoordinator(): JsCoordinator
+  get coordinator(): JsCoordinator
   setInitialized(): void
   cleanup(): Promise<void>
 }

bindings/node/src/lib.rs

@@ -385,18 +385,6 @@ impl JsHookRegistry {
         }
     }
 
-    /// Creates a new **detached** (empty) registry.
-    ///
-    /// Unlike `JsCancellationToken::from_inner`, HookRegistry cannot be cheaply
-    /// cloned or wrapped from a reference, so this always creates an empty
-    /// registry. When Coordinator manages ownership, this should accept
-    /// `Arc<HookRegistry>` to share state.
-    pub fn new_detached() -> Self {
-        Self {
-            inner: Arc::new(amplifier_core::HookRegistry::new()),
-        }
-    }
-
     /// Register a hook handler for the given event name.
     ///
     /// ## Handler signature
@@ -519,28 +507,16 @@ impl JsCoordinator {
         }
     }
 
-    /// Creates a new **detached** (empty) JsHookRegistry.
-    ///
-    /// ⚠️  **Each call returns a brand-new, empty registry** — hooks registered
-    /// on one instance are invisible to the next. This is a known limitation:
-    /// `Coordinator` owns its `HookRegistry` by value, not behind `Arc`, so
-    /// the binding cannot share state across calls.
-    ///
-    /// The method name (`createHookRegistry`) intentionally signals "creates new
-    /// instance" — a getter property would imply referential stability in JS.
+    /// The coordinator's hook registry — shared via `Arc`, not copied.
     ///
-    /// **Workaround:** create a `JsHookRegistry` directly and hold a reference.
-    ///
-    /// Future TODO #1: restructure the kernel to hold `Arc<HookRegistry>` inside
-    /// `Coordinator` so this method can share the same registry instance.
-    #[napi]
-    pub fn create_hook_registry(&self) -> JsHookRegistry {
-        log::warn!(
-            "JsCoordinator::createHookRegistry() — returns a new detached HookRegistry; \
-             hooks registered on one call are NOT visible via the Coordinator's internal \
-             registry. Hold the returned instance directly. (Future TODO #1)"
-        );
-        JsHookRegistry::new_detached()
+    /// Returns a `JsHookRegistry` wrapping the coordinator's real
+    /// `Arc<HookRegistry>` obtained via `hooks_shared()`. Hooks registered
+    /// on the returned instance are visible to the Coordinator and vice versa.
+    #[napi(getter)]
+    pub fn hooks(&self) -> JsHookRegistry {
+        JsHookRegistry {
+            inner: self.inner.hooks_shared(),
+        }
     }
 
     #[napi(getter)]
@@ -579,15 +555,15 @@ impl JsCoordinator {
 /// Lifecycle: `new AmplifierSession(config) → initialize() → execute(prompt) → cleanup()`.
 /// Wires together Coordinator, HookRegistry, and CancellationToken.
 ///
-/// Known limitation: `coordinator` getter creates a separate Coordinator instance
-/// because the kernel Session owns its Coordinator by value, not behind Arc.
-/// Sharing requires restructuring the Rust kernel — tracked as Future TODO #1.
+/// The `coordinator` getter returns the session's real `Arc<Coordinator>`,
+/// and `coordinator.hooks` returns the real `Arc<HookRegistry>` — both
+/// shared, not copied.
 #[napi]
 pub struct JsAmplifierSession {
     inner: Arc<Mutex<amplifier_core::Session>>,
     cached_session_id: String,
     cached_parent_id: Option<String>,
-    cached_config: HashMap<String, serde_json::Value>,
+    cached_coordinator: Option<JsCoordinator>,
 }
 
 #[napi]
@@ -601,20 +577,17 @@ impl JsAmplifierSession {
         let value: serde_json::Value = serde_json::from_str(&config_json)
             .map_err(|e| Error::from_reason(format!("Invalid config JSON: {e}")))?;
 
-        let config = amplifier_core::SessionConfig::from_value(value.clone())
+        let config = amplifier_core::SessionConfig::from_value(value)
             .map_err(|e| Error::from_reason(e.to_string()))?;
 
-        let cached_config: HashMap<String, serde_json::Value> = serde_json::from_value(value)
-            .map_err(|e| Error::from_reason(format!("invalid JSON: {e}")))?;
-
         let session = amplifier_core::Session::new(config, session_id.clone(), parent_id.clone());
         let cached_session_id = session.session_id().to_string();
 
         Ok(Self {
             inner: Arc::new(Mutex::new(session)),
             cached_session_id,
             cached_parent_id: parent_id,
-            cached_config,
+            cached_coordinator: None,
         })
     }
 
@@ -657,42 +630,45 @@ impl JsAmplifierSession {
         }
     }
 
-    /// Creates a new **fresh** JsCoordinator from this session's cached config.
-    ///
-    /// ⚠️  **Each call allocates a new Coordinator** — capabilities registered on
-    /// one instance are invisible to the next. This is a known limitation:
-    /// `Session` owns its `Coordinator` by value, not behind `Arc`, so the
-    /// binding cannot expose the session's live coordinator.
-    ///
-    /// The method name (`createCoordinator`) intentionally signals "creates new
-    /// instance" — a getter property would imply referential stability in JS.
+    /// The session's coordinator — shared via `Arc`, not copied.
     ///
-    /// **Workaround:** call `createCoordinator()` once, hold the returned instance,
-    /// and register capabilities on it before passing it to other APIs.
+    /// Returns a `JsCoordinator` wrapping the session's real `Arc<Coordinator>`.
+    /// Repeated calls return the same underlying coordinator instance.
     ///
-    /// Future TODO #1: restructure the kernel to hold `Arc<Coordinator>` inside
-    /// `Session` so this method can return a handle to the session's actual coordinator.
-    #[napi]
-    pub fn create_coordinator(&self) -> JsCoordinator {
-        log::warn!(
-            "JsAmplifierSession::createCoordinator() — returns a new Coordinator built from \
-             cached config; capabilities registered on one call are NOT visible on the next. \
-             Hold the returned instance directly. (Future TODO #1)"
-        );
-        JsCoordinator {
-            inner: Arc::new(amplifier_core::Coordinator::new(self.cached_config.clone())),
+    /// Takes `&mut self` because the first call caches the coordinator internally.
+    /// This is safe because NAPI JS objects are single-threaded — no concurrent access.
+    #[napi(getter)]
+    pub fn coordinator(&mut self) -> JsCoordinator {
+        if let Some(ref cached) = self.cached_coordinator {
+            return JsCoordinator {
+                inner: Arc::clone(&cached.inner),
+            };
         }
+        // First call: extract the Arc<Coordinator> from the session.
+        // try_lock is safe here — the Mutex is only held during async execute/cleanup.
+        let coord_arc = match self.inner.try_lock() {
+            Ok(session) => session.coordinator_shared(),
+            Err(_) => {
+                log::warn!(
+                    "JsAmplifierSession::coordinator() — session lock held, \
+                     creating coordinator from default config as fallback"
+                );
+                Arc::new(amplifier_core::Coordinator::new(Default::default()))
+            }
+        };
+        let js_coord = JsCoordinator { inner: coord_arc };
+        self.cached_coordinator = Some(JsCoordinator {
+            inner: Arc::clone(&js_coord.inner),
+        });
+        js_coord
     }
 
     #[napi]
     pub fn set_initialized(&self) {
         match self.inner.try_lock() {
             Ok(session) => session.set_initialized(),
-            // State mutation failed — unlike read-only getters, this warrants a warning.
-            // Lock contention only occurs during async cleanup(), so this is unlikely
-            // in practice, but callers should know the mutation didn't happen.
-            Err(_) => eprintln!(
-                "amplifier-core-node: set_initialized() skipped — session lock held (cleanup in progress?)"
+            Err(_) => log::warn!(
+                "JsAmplifierSession::set_initialized() skipped — session lock held (cleanup in progress?)"
             ),
         }
     }