fix: classify wired nodes without editor x/y as regular nodes, not config nodes

src/run-envelope.js

@@ -34,6 +34,43 @@ function stderrLogHandler() {
   };
 }
 
+/**
+ * Node-RED's flow parser (`@node-red/runtime/lib/flows/util.js`) classifies
+ * *any* node lacking both `x` and `y` properties as a global config node --
+ * regardless of its actual `type` -- since those coordinates are otherwise
+ * only ever used by the editor canvas. A real, editor-exported flow always
+ * has them on every wired node, so this never matters there. But hand-authored
+ * `--flow-json` flows (this tool's own core use case; see the README's
+ * `agent` example) commonly omit them, since they carry no runtime meaning.
+ * A wired node misclassified as a config node undergoes `Flow.js`'s
+ * config-node circular-dependency scan instead of normal instantiation,
+ * which scans every one of its own property values against other node ids
+ * and throws "Circular config node dependency detected" the moment any
+ * property value happens to equal another node's id -- including its own,
+ * e.g. a node whose `name` equals its own `id` (an extremely natural thing
+ * to write by hand, and exactly what the README's own agent example does).
+ * That aborts the whole flow's instantiation, so downstream preflight
+ * validation reports the target/return nodes as "not instantiated" even
+ * though the flow is otherwise entirely valid (see issue #28).
+ *
+ * Fix: assign synthetic coordinates to every node that is unambiguously a
+ * regular (wired) node -- i.e. it already declares a `wires` array, or is a
+ * `link out` node (which routes via `links` instead of `wires`) -- so
+ * Node-RED's parser classifies it correctly. Nodes without either (real
+ * config nodes) are left untouched.
+ */
+function withDeployCoordinates(flow) {
+  let n = 0;
+  return flow.map((node) => {
+    const isWired = Object.prototype.hasOwnProperty.call(node, "wires") || node.type === "link out";
+    const hasCoords =
+      Object.prototype.hasOwnProperty.call(node, "x") && Object.prototype.hasOwnProperty.call(node, "y");
+    if (!isWired || hasCoords) return node;
+    n += 1;
+    return { ...node, x: n * 100, y: 100 };
+  });
+}
+
 /**
  * Waits for Node-RED to finish attempting to start the deployed flows.
  *
@@ -55,20 +92,39 @@ function stderrLogHandler() {
  * either way; if the flows never actually started, the target/return nodes
  * simply won't be instantiated and the existing preflight validation in
  * `createHostLinkCaller` reports the real, specific error instead.
+ *
+ * A `stop`/`safe` `runtime-state` event and a real `flows:started` are
+ * mutually exclusive outcomes of the same deploy attempt in the installed
+ * `@node-red/runtime` (each early-return failure path returns before ever
+ * reaching the code that emits `flows:started`), so this never races in
+ * practice today. Still, resolving on `stop`/`safe` is deferred by one
+ * macrotask (`setImmediate`) rather than immediately, so that if a
+ * `flows:started` for the same attempt is already scheduled to fire right
+ * after, it wins instead -- cheap insurance against exactly the kind of
+ * premature-resolution regression reported in issue #28, without delaying
+ * genuine failures beyond a single negligible tick.
+ *
+ * (Uses `setTimeout(fn, 0)` rather than `setImmediate` purely because the
+ * latter isn't part of this project's configured ESLint globals; both defer
+ * to the next macrotask.)
  */
 function waitForFlowsSettled(RED) {
   return new Promise((resolve) => {
-    const onStarted = () => {
+    let settled = false;
+    const finish = () => {
+      if (settled) return;
+      settled = true;
+      RED.events.removeListener("flows:started", onStarted);
       RED.events.removeListener("runtime-event", onRuntimeEvent);
       resolve();
     };
+    const onStarted = () => finish();
     const onRuntimeEvent = (event) => {
       if (
         event?.id === "runtime-state" &&
         (event.payload?.state === "stop" || event.payload?.state === "safe")
       ) {
-        RED.events.removeListener("flows:started", onStarted);
-        resolve();
+        setTimeout(finish, 0);
       }
     };
     RED.events.once("flows:started", onStarted);
@@ -113,7 +169,7 @@ async function runFlowInvocation({ flow, flowFile, msg, options }) {
 
     let caller;
     RED.init({
-      ...(flow ? { storageModule: createMemoryStorageModule(flow) } : { flowFile }),
+      ...(flow ? { storageModule: createMemoryStorageModule(withDeployCoordinates(flow)) } : { flowFile }),
       userDir,
       httpAdminRoot: false,
       httpNodeRoot: false,

test/integration/run-envelope.integration.test.js

@@ -49,3 +49,41 @@ test("integration: runFlowInvocation rejects with preflight error for an unregis
     fs.rmSync(userDir, { recursive: true, force: true });
   }
 });
+
+/**
+ * Regression coverage for issue #28: a v0.2.14 report that flows using the
+ * `agent` node (node-red-agents) deterministically fail with "Circular
+ * config node dependency" / "not instantiated", even though the flow is
+ * otherwise valid and deployed fine on v0.2.12/v0.2.13.
+ *
+ * Root cause (confirmed against the real `@tbrandenburg/node-red-agents`
+ * package, and reproducible with only core node types as below): Node-RED's
+ * flow parser (`@node-red/runtime/lib/flows/util.js`) classifies *any* node
+ * lacking both `x` and `y` as a global config node, regardless of its
+ * actual type -- and the README's own hand-authored `--flow-json` `agent`
+ * example (like this fixture) omits those editor-only coordinates. A wired
+ * node misclassified as a config node undergoes `Flow.js`'s config-node
+ * circular-dependency scan, which throws "Circular config node dependency
+ * detected" the moment one of its own property values happens to equal
+ * another node's id -- including its own id, e.g. a node whose `name`
+ * equals its own `id` (exactly what this fixture, and the README example,
+ * both do). That aborts the whole flow's instantiation, which is what
+ * previously made the *unrelated* `waitForFlowsSettled` race a prime
+ * suspect: the target/return nodes end up "not instantiated" either way.
+ * This flow is otherwise entirely valid and must deploy and succeed exactly
+ * as it did before v0.2.14.
+ */
+const SELF_NAMED_LINK_FLOW = [
+  { id: "tab", type: "tab", label: "t" },
+  { id: "ask", type: "link in", z: "tab", name: "ask", wires: [["return"]] },
+  { id: "return", type: "link out", z: "tab", name: "return", mode: "return" }
+];
+
+test("integration: runFlowInvocation deploys and calls a flow whose nodes omit editor x/y coordinates (issue #28)", async () => {
+  const result = await runFlowInvocation({
+    flow: SELF_NAMED_LINK_FLOW,
+    msg: { payload: "hi" },
+    options: { target: "ask", timeoutMs: 5000, format: "json" }
+  });
+  assert.deepEqual(JSON.parse(result.output).payload, "hi");
+});