Fix node resolution and Vue reactivity in useReplicatedModels for composite models

[Copilot]

When using multiple models, each tree's ChangeListener was resolving nodes only from its own tree and wrapping them inconsistently, causing "node not found" errors and breaking Vue's reactivity cache.

Changes

• Modified ChangeListener in MutableModelTreeJsImpl.kt

  • Accept CompositeModel parameter to resolve nodes across all trees
  • Use model.tryResolveNode(nodeId) instead of NodeInMutableModel(tree, nodeId)
  • Wrap all nodes consistently with AutoTransactionsNode(node, model) to maintain cache identity

• Added Kotlin tests in MutableModelTreeJsTest.kt

  • Test composite model resolves nodes from multiple trees
  • Test change listeners work across all models without "node not found" errors
  • Test consistent node wrappers for same node reference (critical for Vue reactivity cache)

• Added TypeScript regression tests in useReplicatedModels.test.ts

  • Single model reactivity (backwards compatibility)
  • Multiple models working together
  • Change propagation across composite models

Technical Detail

Before:

fun nodeIdToInode(nodeId: INodeReference): INodeJS {
    return toNodeJs(NodeInMutableModel(tree, nodeId).asLegacyNode())
}

After:

fun nodeIdToInode(nodeId: INodeReference): INodeJS {
    val node = model.tryResolveNode(nodeId) ?: // fallback to tree
        return toNodeJs(NodeInMutableModel(tree, nodeId).asLegacyNode())
    return toNodeJs(AutoTransactionsNode(node, model).asLegacyNode())
}

This ensures the same node reference always produces the same wrapper object, which Vue's cache requires for reactivity tracking.

Testing

The fix is verified at both layers:

• Kotlin layer (MutableModelTreeJsTest.kt): Tests the core node resolution and change listener behavior with real composite models
• TypeScript layer (useReplicatedModels.test.ts): Tests the Vue.js integration and reactivity behavior

Original prompt

Problem

The useReplicatedModels implementation has critical bugs causing:

1. Node not found exceptions when working with multiple models
2. Broken Vue.js reactivity - changes to model data don't trigger Vue component re-renders

Root Cause

In MutableModelTreeJsImpl.kt, when multiple trees are used, each tree gets its own ChangeListener that wraps nodes using only that specific tree:

internal class ChangeListener(private val tree: IMutableModelTree, private val changeCallback: (ChangeJS) -> Unit) :
    IGenericMutableModelTree.Listener<INodeReference> {

    fun nodeIdToInode(nodeId: INodeReference): INodeJS {
        return toNodeJs(NodeInMutableModel(tree, nodeId).asLegacyNode())
    }

The problem: When a change occurs in one tree, the ChangeListener tries to resolve the node reference using only its own tree. But in a composite model, nodes might belong to different trees, causing:

1. Node not found: The node reference can't be resolved in that specific tree
2. Broken reactivity: The same node gets wrapped differently depending on which ChangeListener processes it. Vue's cache uses node references as keys, but NodeInMutableModel(treeA, nodeId) and NodeInMutableModel(treeB, nodeId) are different objects with different identities, breaking cache lookups.

Additionally, even with a single model, the backwards-compatibility wrapper in useReplicatedModel goes through the composite model path, which changes how nodes are wrapped compared to the old implementation.

Required Fixes

1. Fix ChangeListener to use CompositeModel for node resolution

File: model-client/src/jsMain/kotlin/org/modelix/model/client2/MutableModelTreeJsImpl.kt

Modify ChangeListener to accept and use the CompositeModel for node resolution:

internal class ChangeListener(
    private val tree: IMutableModelTree,
    private val model: CompositeModel,  // Add this parameter
    private val changeCallback: (ChangeJS) -> Unit
) : IGenericMutableModelTree.Listener<INodeReference> {

    fun nodeIdToInode(nodeId: INodeReference): INodeJS {
        // Use the composite model to resolve nodes from any tree
        val node = model.tryResolveNode(nodeId)
        if (node == null) {
            // Log or handle the case where node cannot be resolved
            console.log("Warning: Could not resolve node $nodeId in composite model")
            // Fall back to the old behavior for this tree
            return toNodeJs(NodeInMutableModel(tree, nodeId).asLegacyNode())
        }
        return toNodeJs(node.asLegacyNode())
    }

    // ... rest of the class
}

And update the initialization in MutableModelTreeJsImpl:

private val changeListeners = trees.map { tree ->
    ChangeListener(tree, model) { change ->  // Pass the model
        changeHandlers.forEach { it(change) }
    }.also { tree.addListener(it) }
    // TODO missing removeListener call
}

2. Add regression tests

File: vue-model-api/src/useReplicatedModels.test.ts

Add comprehensive tests to verify:

1. Single model reactivity still works (backwards compatibility)
2. Multiple models work correctly
3. Changes in one model trigger Vue reactivity correctly
4. Node references are resolved correctly across models

import { org } from "@modelix/model-client";
import { toRoleJS } from "@modelix/ts-model-api";
import { watchEffect, computed, ref } from "vue";
import { useModelClient } from "./useModelClient";
import { useReplicatedModel, useReplicatedModels } from "./useReplicatedModels";
import IdSchemeJS = org.modelix.model.client2.IdSchemeJS;

type ClientJS = org.modelix.model.client2.ClientJS;
type ReplicatedModelJS = org.modelix.model.client2.ReplicatedModelJS;
type ReplicatedModelParameters = org.modelix.model.client2.ReplicatedModelParameters;

const { loadModelsFromJson } = org.modelix.model.client2;

describe("useReplicatedModels reactivity", () => {
  test("single model reactivity works with useReplicatedModel wrapper", (done) => {
    class SuccessfulClientJS {
      startReplicatedModels(
        parameters: ReplicatedModelParameters[],
      ): Promise<ReplicatedModelJS> {
        const branchId = parameters[0].branchId;
        const rootNode = loadModelsFromJson([JSON.stringify({ root: {} })]);
        rootNode.setPropertyValue(toRoleJS("testProp"), "initialValue");

        let listener: ((change: any) => void) | null = null;

        const branch = {
          rootNode,
          getRootNodes: () => [rootNode],
          addListener: (fn: (change: any) => void) => {
            listener = fn;
          },
          removeListener: jest.fn(),
          resolveNode: jest.fn(),
        };

        const replicatedModel = {
          getBranch: () => branch,
          dispose: jest.fn(),
          getCurrentVersionInformation: jest.fn(),
        } as unknown as ReplicatedModelJS;

        // Simulate a property change after a delay
        setTimeout((...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created as a result of the following prompt from Copilot chat.*
> ## Problem
> 
> The `useReplicatedModels` implementation has critical bugs causing:
> 1. **Node not found exceptions** when working with multiple models
> 2. **Broken Vue.js reactivity** - changes to model data don't trigger Vue component re-renders
> 
> ## Root Cause
> 
> In `MutableModelTreeJsImpl.kt`, when multiple trees are used, each tree gets its own `ChangeListener` that wraps nodes using only that specific tree:
> 
> ```kotlin
> internal class ChangeListener(private val tree: IMutableModelTree, private val changeCallback: (ChangeJS) -> Unit) :
>     IGenericMutableModelTree.Listener<INodeReference> {
> 
>     fun nodeIdToInode(nodeId: INodeReference): INodeJS {
>         return toNodeJs(NodeInMutableModel(tree, nodeId).asLegacyNode())
>     }
> ```
> 
> **The problem:** When a change occurs in one tree, the `ChangeListener` tries to resolve the node reference using only its own tree. But in a composite model, nodes might belong to different trees, causing:
> 
> 1. **Node not found**: The node reference can't be resolved in that specific tree
> 2. **Broken reactivity**: The same node gets wrapped differently depending on which `ChangeListener` processes it. Vue's cache uses node references as keys, but `NodeInMutableModel(treeA, nodeId)` and `NodeInMutableModel(treeB, nodeId)` are different objects with different identities, breaking cache lookups.
> 
> Additionally, even with a single model, the backwards-compatibility wrapper in `useReplicatedModel` goes through the composite model path, which changes how nodes are wrapped compared to the old implementation.
> 
> ## Required Fixes
> 
> ### 1. Fix ChangeListener to use CompositeModel for node resolution
> 
> **File: `model-client/src/jsMain/kotlin/org/modelix/model/client2/MutableModelTreeJsImpl.kt`**
> 
> Modify `ChangeListener` to accept and use the `CompositeModel` for node resolution:
> 
> ```kotlin
> internal class ChangeListener(
>     private val tree: IMutableModelTree,
>     private val model: CompositeModel,  // Add this parameter
>     private val changeCallback: (ChangeJS) -> Unit
> ) : IGenericMutableModelTree.Listener<INodeReference> {
> 
>     fun nodeIdToInode(nodeId: INodeReference): INodeJS {
>         // Use the composite model to resolve nodes from any tree
>         val node = model.tryResolveNode(nodeId)
>         if (node == null) {
>             // Log or handle the case where node cannot be resolved
>             console.log("Warning: Could not resolve node $nodeId in composite model")
>             // Fall back to the old behavior for this tree
>             return toNodeJs(NodeInMutableModel(tree, nodeId).asLegacyNode())
>         }
>         return toNodeJs(node.asLegacyNode())
>     }
> 
>     // ... rest of the class
> }
> ```
> 
> And update the initialization in `MutableModelTreeJsImpl`:
> 
> ```kotlin
> private val changeListeners = trees.map { tree ->
>     ChangeListener(tree, model) { change ->  // Pass the model
>         changeHandlers.forEach { it(change) }
>     }.also { tree.addListener(it) }
>     // TODO missing removeListener call
> }
> ```
> 
> ### 2. Add regression tests
> 
> **File: `vue-model-api/src/useReplicatedModels.test.ts`**
> 
> Add comprehensive tests to verify:
> 1. Single model reactivity still works (backwards compatibility)
> 2. Multiple models work correctly
> 3. Changes in one model trigger Vue reactivity correctly
> 4. Node references are resolved correctly across models
> 
> ```typescript
> import { org } from "@modelix/model-client";
> import { toRoleJS } from "@modelix/ts-model-api";
> import { watchEffect, computed, ref } from "vue";
> import { useModelClient } from "./useModelClient";
> import { useReplicatedModel, useReplicatedModels } from "./useReplicatedModels";
> import IdSchemeJS = org.modelix.model.client2.IdSchemeJS;
> 
> type ClientJS = org.modelix.model.client2.ClientJS;
> type ReplicatedModelJS = org.modelix.model.client2.ReplicatedModelJS;
> type ReplicatedModelParameters = org.modelix.model.client2.ReplicatedModelParameters;
> 
> const { loadModelsFromJson } = org.modelix.model.client2;
> 
> describe("useReplicatedModels reactivity", () => {
>   test("single model reactivity works with useReplicatedModel wrapper", (done) => {
>     class SuccessfulClientJS {
>       startReplicatedModels(
>         parameters: ReplicatedModelParameters[],
>       ): Promise<ReplicatedModelJS> {
>         const branchId = parameters[0].branchId;
>         const rootNode = loadModelsFromJson([JSON.stringify({ root: {} })]);
>         rootNode.setPropertyValue(toRoleJS("testProp"), "initialValue");
> 
>         let listener: ((change: any) => void) | null = null;
> 
>         const branch = {
>           rootNode,
>           getRootNodes: () => [rootNode],
>           addListener: (fn: (change: any) => void) => {
>             listener = fn;
>           },
>           removeListener: jest.fn(),
>           resolveNode: jest.fn(),
>         };
> 
>         const replicatedModel = {
>           getBranch: () => branch,
>           dispose: jest.fn(),
>           getCurrentVersionInformation: jest.fn(),
>         } as unknown as ReplicatedModelJS;
> 
>         // Simulate a property change after a delay
>         setTimeout(() => {
>           rootNode.setPropertyValue(toRoleJS("testProp"), "changedValue");
>           if (listener) {
>             listener({
>               node: rootNode,
>               role: "testProp",
>               constructor: { name: "PropertyChanged" },
>             });
>           }
>         }, 50);
> 
>         return Promise.resolve(replicatedModel);
>       }
>     }
> 
>     const { client } = useModelClient("anURL", () =>
>       Promise.resolve(new SuccessfulClientJS() as unknown as ClientJS),
>     );
> 
>     const { rootNode } = useReplicatedModel(
>       client,
>       "aRepository",
>       "aBranch",
>       IdSchemeJS.MODELIX,
>     );
> 
>     let changeDetected = false;
> 
>     watchEffect(() => {
>       if (rootNode.value !== null) {
>         const propValue = rootNode.value.getPropertyValue(toRoleJS("testProp"));
>         if (propValue === "changedValue") {
>           changeDetected = true;
>           expect(changeDetected).toBe(true);
>           done();
>         }
>       }
>     });
>   });
> 
>   test("multiple models can be used together", (done) => {
>     class SuccessfulClientJS {
>       startReplicatedModels(
>         parameters: ReplicatedModelParameters[],
>       ): Promise<ReplicatedModelJS> {
>         const rootNodes = parameters.map((params) => {
>           const node = loadModelsFromJson([JSON.stringify({ root: {} })]);
>           node.setPropertyValue(toRoleJS("modelId"), params.branchId);
>           return node;
>         });
> 
>         const branch = {
>           rootNode: rootNodes[0],
>           getRootNodes: () => rootNodes,
>           addListener: jest.fn(),
>           removeListener: jest.fn(),
>           resolveNode: jest.fn(),
>         };
> 
>         const replicatedModel = {
>           getBranch: () => branch,
>           dispose: jest.fn(),
>           getCurrentVersionInformation: jest.fn(),
>         } as unknown as ReplicatedModelJS;
> 
>         return Promise.resolve(replicatedModel);
>       }
>     }
> 
>     const { client } = useModelClient("anURL", () =>
>       Promise.resolve(new SuccessfulClientJS() as unknown as ClientJS),
>     );
> 
>     const modelsParams = ref([
>       new org.modelix.model.client2.ReplicatedModelParameters(
>         "repo1",
>         "branch1",
>         IdSchemeJS.MODELIX,
>       ),
>       new org.modelix.model.client2.ReplicatedModelParameters(
>         "repo2",
>         "branch2",
>         IdSchemeJS.MODELIX,
>       ),
>     ]);
> 
>     const { rootNodes } = useReplicatedModels(client, modelsParams);
> 
>     watchEffect(() => {
>       if (rootNodes.value.length === 2) {
>         expect(rootNodes.value[0].getPropertyValue(toRoleJS("modelId"))).toBe("branch1");
>         expect(rootNodes.value[1].getPropertyValue(toRoleJS("modelId"))).toBe("branch2");
>         done();
>       }
>     });
>   });
> 
>   test("changes in composite model trigger reactivity", (done) => {
>     class SuccessfulClientJS {
>       startReplicatedModels(
>         parameters: ReplicatedModelParameters[],
>       ): Promise<ReplicatedModelJS> {
>         const rootNodes = parameters.map((params) => {
>           const node = loadModelsFromJson([JSON.stringify({ root: {} })]);
>           node.setPropertyValue(toRoleJS("counter"), "0");
>           return node;
>         });
> 
>         let listener: ((change: any) => void) | null = null;
> 
>         const branch = {
>           rootNode: rootNodes[0],
>           getRootNodes: () => rootNodes,
>           addListener: (fn: (change: any) => void) => {
>             listener = fn;
>           },
>           removeListener: jest.fn(),
>           resolveNode: jest.fn(),
>         };
> 
>         const replicatedModel = {
>           getBranch: () => branch,
>           dispose: jest.fn(),
>           getCurrentVersionInformation: jest.fn(),
>         } as unknown as ReplicatedModelJS;
> 
>         // Simulate property change in second model
>         setTimeout(() => {
>           rootNodes[1].setPropertyValue(toRoleJS("counter"), "1");
>           if (listener) {
>             listener({
>               node: rootNodes[1],
>               role: "counter",
>               constructor: { name: "PropertyChanged" },
>             });
>           }
>         }, 50);
> 
>         return Promise.resolve(replicatedModel);
>       }
>     }
> 
>     const { client } = useModelClient("anURL", () =>
>       Promise.resolve(new SuccessfulClientJS() as unknown as ClientJS),
>     );
> 
>     const modelsParams = ref([
>       new org.modelix.model.client2.ReplicatedModelParameters(
>         "repo1",
>         "branch1",
>         IdSchemeJS.MODELIX,
>       ),
>       new org.modelix.model.client2.ReplicatedModelParameters(
>         "repo2",
>         "branch2",
>         IdSchemeJS.MODELIX,
>       ),
>     ]);
> 
>     const { rootNodes } = useReplicatedModels(client, modelsParams);
> 
>     const computedValue = computed(() => {
>       if (rootNodes.value.length >= 2) {
>         return rootNodes.value[1].getPropertyValue(toRoleJS("counter"));
>       }
>       return null;
>     });
> 
>     watchEffect(() => {
>       if (computedValue.value === "1") {
>         expect(computedValue.value).toBe("1");
>         done();
>       }
>     });
>   });
> });
> ```
> 
> ## Acceptance Criteria
> 
> 1. All existing tests pass
> 2. New regression tests pass
> 3. Single model usage through `useReplicatedModel` maintains backwards compatibility
> 4. Multiple models work correctly with proper node resolution
> 5. Vue reactivity triggers correctly for changes in any model
> 
> ## Assignee
> 
> @abstraktor

<!-- START COPILOT CODING AGENT TIPS -->
---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).

[github-actions]

Test Results

160 files   -  80  160 suites   - 80   47m 1s ⏱️ - 1m 48s
850 tests  - 522  841 ✅  - 520  9 💤  - 2  0 ❌ ±0 
860 runs   - 522  851 ✅  - 520  9 💤  - 2  0 ❌ ±0 

Results for commit 99f3cbc. ± Comparison against base commit a7e90a0.

♻️ This comment has been updated with latest results.

[abstraktor]

@copilot fix the test and make sure it is indeed verifying this misbehavior to be gone. consider testing it in MutableModelTreeJsTest

[Copilot]

@copilot fix the test and make sure it is indeed verifying this misbehavior to be gone. consider testing it in MutableModelTreeJsTest

Added comprehensive Kotlin tests in MutableModelTreeJsTest (commit 99f3cbc) that directly verify the bug fix:

1. compositeModelResolvesNodesFromMultipleTrees - Tests that nodes from different models can be resolved correctly through the composite model
2. compositeModelChangeListenerResolvesNodesFromAllTrees - Verifies change listeners can handle events from all models without "node not found" errors
3. compositeModelReturnsConsistentNodeWrappersForSameNode - Ensures the same node reference always returns equal node wrappers (critical for Vue reactivity cache)

These tests verify the actual behavior at the Kotlin layer where the fix was applied, ensuring the CompositeModel-based node resolution works correctly.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• artifacts.itemis.cloud
  • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -XX:MaxMetaspaceSize=2g -XX:+HeapDumpOnOutOfMemoryError -Xms256m -Xmx1024m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)