From eb3750c3cb439417d783160a5ef0fdd56ac6934c Mon Sep 17 00:00:00 2001 From: Tom Brandenburg Date: Sun, 6 Sep 2026 20:18:32 +0200 Subject: [PATCH] fix(docker): revert NODE_RED_CLI_DEFAULT_USERDIR; add /data auto-probe + --docker-userdir override Reverts the NODE_RED_CLI_DEFAULT_USERDIR env-var convention from #31/#32, which required community image authors to opt in and was confirmed dead-on-arrival for the motivating agentic-workflow-dev-env image. Replaces it with: - a best-effort /data auto-probe (sandbox entrypoint only) that validates node_modules/* and node_modules/@*/* (following symlinks) for a package.json declaring a "node-red" key before trusting the path - an explicit --docker-userdir CLI flag as a reliable, zero-guessing override for any convention - a 4-level userDir precedence: --user-dir > --docker-userdir > auto-probed /data > ephemeral default - a clearer --node-modules preflight error when combined with --docker-userdir/auto-probe without an explicit --user-dir Closes #33 --- README.md | 36 ++-- bin/node-red-cli-sandbox-entry.js | 15 +- bin/node-red-cli.js | 37 +++- src/run-envelope.js | 157 +++++++++++---- test/e2e/agentic-workflow-dev-env.e2e.test.js | 149 ++++++++++++++ test/integration/docker.integration.test.js | 161 ++++++++++++--- .../run-envelope.integration.test.js | 53 ++--- test/unit/run-envelope.unit.test.js | 186 ++++++++++++------ 8 files changed, 611 insertions(+), 183 deletions(-) create mode 100644 test/e2e/agentic-workflow-dev-env.e2e.test.js diff --git a/README.md b/README.md index d7b795c..d6e0518 100644 --- a/README.md +++ b/README.md @@ -290,14 +290,20 @@ deterministic **named Docker volume** (derived from the `--user-dir` value) mounted inside the container, never a host bind mount — so "no stray host files" holds even for persistent installs. -For images/derived images that pre-install Node-RED node packages into -their own conventional userDir, setting the `NODE_RED_CLI_DEFAULT_USERDIR` -environment variable (inside the image, e.g. via `ENV`) to that path lets -`--docker` discover it automatically whenever `--user-dir` isn't given — -that directory is used as `userDir` and, like an explicit `--user-dir`, -never deleted afterward. If the path doesn't exist or isn't a directory, -`--docker` logs a warning and falls back to the normal ephemeral `userDir` -rather than failing the invocation. +Without an explicit `--user-dir`, `--docker` also auto-probes the +container's own `/data` for a userDir a community image already +pre-populated with its own Node-RED node packages (e.g. the motivating +[`ghcr.io/tbrandenburg/agentic-workflow-dev-env`](https://github.com/tbrandenburg/agentic-workflow-dev-env), +which sets `NODE_RED_HOME=/data`) — validated by scanning `/data/node_modules` +(including scoped `@scope/*` packages) for any `package.json` declaring a +`"node-red"` key. This is inherently best-effort: an unrelated `/data` that +happens to contain such a package is a (rare) false positive, and a real +userDir laid out differently is a false negative that silently falls back +to the ephemeral default. For a reliable, explicit alternative, pass +`--docker-userdir ` to name the in-container directory directly — +it takes precedence over the auto-probe (but is itself still overridden by +an explicit `--user-dir`). Whichever wins, that directory is used as +`userDir` and, like an explicit `--user-dir`, never deleted afterward. Fails fast with a clear `node-red-cli: docker unavailable: ...` error if the Docker CLI/daemon isn't reachable, or `node-red-cli: docker build @@ -326,12 +332,11 @@ echo '{"payload":"Summarize this repo in one sentence.","cwd":"/repo"}' \ The same flow runs sandboxed via `--docker ` against an image that already ships `opencode` + `node-red-agents`, e.g. -[`ghcr.io/tbrandenburg/agentic-workflow-dev-env`](https://github.com/tbrandenburg/agentic-workflow-dev-env) -(`--network` is required for network access, since the agent calls out to -its own API; `--node-modules`/`--user-dir` are still required too, since -Node-RED only discovers node types from a userDir it actually loaded, and -`--docker` doesn't yet reuse an image's own pre-populated default userDir — -see [#31](https://github.com/tbrandenburg/node-red-cli/issues/31)): +[`ghcr.io/tbrandenburg/agentic-workflow-dev-env`](https://github.com/tbrandenburg/agentic-workflow-dev-env), +which pre-installs its node packages into `/data` (`NODE_RED_HOME=/data`) — +exactly the layout the `/data` auto-probe discovers automatically, with +no `--node-modules`/`--user-dir` needed (`--network` is still required for +network access, since the agent calls out to its own API): ```bash echo '{"payload":"Summarize this repo in one sentence.","cwd":"/repo"}' \ @@ -343,8 +348,7 @@ echo '{"payload":"Summarize this repo in one sentence.","cwd":"/repo"}' \ "cwd":"cwd","cwdType":"msg","wires":[["return"],[]]}, {"id":"return","type":"link out","z":"tab","name":"return","mode":"return"} ]' ask --docker ghcr.io/tbrandenburg/agentic-workflow-dev-env:latest \ - --node-modules @tbrandenburg/node-red-agents --user-dir --network \ - --timeout=120000 --format=json + --network --timeout=120000 --format=json ``` ## Host API 🛠️ diff --git a/bin/node-red-cli-sandbox-entry.js b/bin/node-red-cli-sandbox-entry.js index 09ebce0..cbc9dca 100644 --- a/bin/node-red-cli-sandbox-entry.js +++ b/bin/node-red-cli-sandbox-entry.js @@ -8,11 +8,14 @@ * the shared `runFlowInvocation` (the exact same logic the host CLI uses * for its non-Docker path), and writes the formatted result to stdout. * - * The `NODE_RED_CLI_DEFAULT_USERDIR` env-var convention (see #31), which - * lets an image's own pre-populated default userDir be discovered when - * `--user-dir` isn't given, is resolved entirely inside the shared - * `runFlowInvocation` (`src/run-envelope.js`) -- nothing to do here beyond - * the existing pass-through of `envelope.options`. + * Sets `options.probeContainerDefault: true` before delegating to + * `runFlowInvocation`, so its `/data` auto-probe (see #33, + * `resolveContainerDefaultUserDir` in `src/run-envelope.js`) is only ever + * attempted here -- inside the container -- and never on the host CLI + * path, since `/data` has no reserved meaning outside a container. It only + * takes effect as a fallback: an explicit `userDir` (named-volume mount) or + * `dockerUserDir` (`--docker-userdir`) already present on `envelope.options` + * still wins, per `resolveEffectiveUserDir`'s precedence. */ const { runFlowInvocation } = require("../src/run-envelope"); @@ -41,7 +44,7 @@ async function main() { const { output } = await runFlowInvocation({ flow: envelope.flow, msg: envelope.msg, - options: envelope.options || {} + options: { ...(envelope.options || {}), probeContainerDefault: true } }); process.stdout.write(`${output}\n`); } catch (error) { diff --git a/bin/node-red-cli.js b/bin/node-red-cli.js index c5f39c5..8f84944 100755 --- a/bin/node-red-cli.js +++ b/bin/node-red-cli.js @@ -74,9 +74,16 @@ const HELP_TEXT = [ "network access independent of installing any package), --read-only", "rootfs with a /tmp tmpfs, --cap-drop=ALL, --security-opt=no-new-privileges.", "When combined with --user-dir, persistence uses a named Docker volume,", - "never a host bind mount. Image authors can set", - "NODE_RED_CLI_DEFAULT_USERDIR= so a pre-installed userDir is", - "discovered automatically when --user-dir isn't given.", + "never a host bind mount. Without --user-dir, the container's own /data", + "is auto-probed for a pre-populated Node-RED userDir (best-effort; see", + "--docker-userdir for a reliable, explicit alternative).", + "", + "--docker-userdir tells --docker to use (a directory inside", + "the container, e.g. one a base image already pre-installs Node-RED node", + "packages into) as the userDir, instead of the default ephemeral tmpdir or", + "the best-effort /data auto-probe. Ignored outside --docker mode.", + "Precedence: --user-dir > --docker-userdir > the /data auto-probe > the", + "ephemeral default.", "", "Example:", ' echo \'{"payload":{"x":4,"y":5}}\' | node-red-cli flows.json calculate', @@ -197,10 +204,18 @@ async function run(args, options) { const persistentUserDir = resolveUserDir(options.userDir); if (nodeModules.length > 0 && !persistentUserDir) { - console.error( - "node-red-cli: --node-modules requires an explicit --user-dir (a persistent directory); " + - "using it with the default ephemeral userDir would reinstall from npm on every run" - ); + if (options.dockerUserdir || options.docker) { + console.error( + "node-red-cli: --node-modules requires an explicit --user-dir (a persistent directory); " + + "--docker-userdir and the best-effort /data auto-probe are not guaranteed persistent, " + + "so installing into them would just reinstall from npm on every run" + ); + } else { + console.error( + "node-red-cli: --node-modules requires an explicit --user-dir (a persistent directory); " + + "using it with the default ephemeral userDir would reinstall from npm on every run" + ); + } process.exitCode = 1; return; } @@ -226,7 +241,8 @@ async function run(args, options) { timeoutMs: options.timeout, format: options.format, nodeModules, - userDir: volumeName ? CONTAINER_USER_DIR : undefined + userDir: volumeName ? CONTAINER_USER_DIR : undefined, + dockerUserDir: options.dockerUserdir } }; @@ -302,6 +318,11 @@ program "run the invocation sandboxed in a disposable Docker container; bare = cached default image, " + "'' = explicit image (installed into if missing), '@path'/URL = build from a Dockerfile" ) + .option( + "--docker-userdir ", + "in --docker mode, use (inside the container) as the userDir instead of the default " + + "ephemeral tmpdir or the best-effort /data auto-probe; ignored outside --docker mode" + ) .option( "--network", "enable network access in --docker mode, independent of --node-modules (default: --network none)" diff --git a/src/run-envelope.js b/src/run-envelope.js index 47291f3..b17bc8d 100644 --- a/src/run-envelope.js +++ b/src/run-envelope.js @@ -133,31 +133,110 @@ function waitForFlowsSettled(RED) { } /** - * Resolves the image/host-provided default `userDir` from the - * `NODE_RED_CLI_DEFAULT_USERDIR` environment variable (see #31): community - * Docker images that pre-install Node-RED node packages into their own - * conventional userDir can set this variable so `--docker` (without an - * explicit `--user-dir`) discovers it automatically. Returns `undefined` if - * unset. Fails open, not closed: if the path doesn't exist, isn't a - * directory, or isn't accessible, logs a one-line stderr warning and returns - * `undefined` so the caller falls back to its normal ephemeral tmpdir, - * rather than aborting the invocation. + * True if `dirPath` exists and, following symlinks (npm frequently + * symlinks packages, e.g. in workspace/monorepo installs), is a directory. + * Never throws: any stat failure (missing path, broken symlink, etc.) + * resolves to `false`. */ -function resolveDefaultUserDir() { - const configuredPath = process.env.NODE_RED_CLI_DEFAULT_USERDIR; - if (!configuredPath) return undefined; +function isDirectory(dirPath) { + try { + return fs.statSync(dirPath).isDirectory(); + } catch { + return false; + } +} +/** + * Returns every immediate subdirectory of `node_modules`, including one + * level of scoped-package expansion (`@scope/*`), as a flat list of + * absolute directory paths. Follows symlinks (see `isDirectory`). Never + * throws: an unreadable/missing `node_modules` (or scope dir) simply + * contributes no candidates. + */ +function listNodeModuleDirs(nodeModulesDir) { + let entries; try { - if (fs.statSync(configuredPath).isDirectory()) return configuredPath; - console.error( - `node-red-cli: NODE_RED_CLI_DEFAULT_USERDIR='${configuredPath}' is not usable (not a directory), falling back to an ephemeral userDir` - ); - } catch (error) { - console.error( - `node-red-cli: NODE_RED_CLI_DEFAULT_USERDIR='${configuredPath}' is not usable (${error.message}), falling back to an ephemeral userDir` - ); + entries = fs.readdirSync(nodeModulesDir); + } catch { + return []; + } + + return entries.flatMap((name) => { + const entryPath = path.join(nodeModulesDir, name); + if (!name.startsWith("@")) return isDirectory(entryPath) ? [entryPath] : []; + if (!isDirectory(entryPath)) return []; + + let scopedNames; + try { + scopedNames = fs.readdirSync(entryPath); + } catch { + return []; + } + return scopedNames.map((scoped) => path.join(entryPath, scoped)).filter(isDirectory); + }); +} + +/** True if `packageDir/package.json` exists, parses, and declares a `"node-red"` key. */ +function isNodeRedPackage(packageDir) { + try { + const pkg = JSON.parse(fs.readFileSync(path.join(packageDir, "package.json"), "utf8")); + return Boolean(pkg["node-red"]); + } catch { + return false; } - return undefined; +} + +/** + * Auto-probes `baseDir` (default `/data`, the conventional mount point of + * the motivating `ghcr.io/tbrandenburg/agentic-workflow-dev-env` image, see + * issue #33) for a Node-RED userDir a community image pre-populated with + * its own node packages. Returns `baseDir` when it exists, is a directory, + * and at least one direct or scoped (`@scope/*`) child of its + * `node_modules` declares a `"node-red"` key in `package.json`; returns + * `undefined` otherwise. + * + * Best-effort by design: a real userDir with unrelated packages under + * `node_modules` (false negative) or a `/data` that merely happens to + * contain an unrelated `"node-red"`-keyed package (false positive) are both + * possible; `--docker-userdir ` is the reliable, explicit alternative + * when this heuristic doesn't fit an image. Never throws: any missing or + * unreadable path along the way resolves to "not usable". + * + * Only meaningful inside a container (`/data` has no reserved meaning on + * the host), so this is only ever called from the sandbox entrypoint + * (`bin/node-red-cli-sandbox-entry.js`), never from the host CLI path. + */ +function resolveContainerDefaultUserDir(baseDir = "/data") { + if (!isDirectory(baseDir)) return undefined; + const candidateDirs = listNodeModuleDirs(path.join(baseDir, "node_modules")); + return candidateDirs.some(isNodeRedPackage) ? baseDir : undefined; +} + +/** + * Resolves which `userDir` source wins, in order of precedence (see #33): + * + * 1. `userDir` -- host-managed, explicit `--user-dir` (or its container + * named-volume mount path). + * 2. `dockerUserDir` -- explicit `--docker-userdir ` passthrough. + * 3. the auto-probed `/data` default (see `resolveContainerDefaultUserDir`), + * only attempted when `probeContainerDefault` is set (sandbox entrypoint + * only). + * 4. `undefined` -- caller falls back to an ephemeral tmpdir. + * + * The first three are all treated as persistent (never removed afterward); + * only the ephemeral tmpdir fallback is managed/cleaned up by the caller. + * + * `probeBaseDir` overrides the auto-probed path (defaults to `/data`) -- + * only ever used by tests; real callers always probe the real `/data`. + */ +function resolveEffectiveUserDir({ userDir, dockerUserDir, probeContainerDefault, probeBaseDir } = {}) { + if (userDir) return { userDir, persistent: true }; + if (dockerUserDir) return { userDir: dockerUserDir, persistent: true }; + if (probeContainerDefault) { + const probed = resolveContainerDefaultUserDir(probeBaseDir); + if (probed) return { userDir: probed, persistent: true }; + } + return { userDir: undefined, persistent: false }; } /** @@ -172,15 +251,14 @@ function resolveDefaultUserDir() { * entrypoint (`bin/node-red-cli-sandbox-entry.js`, `--docker` path), so both * execute the exact same runtime logic. * - * `options.userDir`, when set, is treated as a persistent directory and is - * never removed afterward (host: an explicit `--user-dir`; container: the - * fixed mount path of a named Docker volume). When omitted, and the - * `NODE_RED_CLI_DEFAULT_USERDIR` environment variable points at an existing - * directory (see `resolveDefaultUserDir`), that directory is used instead — - * also treated as persistent and never removed afterward, letting a Docker - * image's own pre-populated default userDir be discovered automatically - * (see #31). Otherwise an ephemeral tmpdir is created and removed again - * after the call. + * `userDir` resolution follows `resolveEffectiveUserDir`'s precedence: + * `options.userDir` (host: an explicit `--user-dir`; container: the fixed + * mount path of a named Docker volume) > `options.dockerUserDir` (explicit + * `--docker-userdir ` passthrough) > the auto-probed `/data` default + * (see `resolveContainerDefaultUserDir`, only attempted when + * `options.probeContainerDefault` is set -- sandbox entrypoint only) > an + * ephemeral tmpdir created fresh and removed again after the call. The + * first three are all treated as persistent and never removed afterward. */ async function runFlowInvocation({ flow, flowFile, msg, options }) { const { @@ -189,14 +267,14 @@ async function runFlowInvocation({ flow, flowFile, msg, options }) { timeoutMs = 5000, format = "plain", nodeModules = [], - userDir: fixedUserDir + userDir: fixedUserDir, + dockerUserDir, + probeContainerDefault } = options; - const persistentUserDir = Boolean(fixedUserDir); - const imageDefaultUserDir = !persistentUserDir ? resolveDefaultUserDir() : undefined; - const userDir = - fixedUserDir || imageDefaultUserDir || fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-")); - const managedUserDir = !persistentUserDir && !imageDefaultUserDir; + const resolved = resolveEffectiveUserDir({ userDir: fixedUserDir, dockerUserDir, probeContainerDefault }); + const userDir = resolved.userDir || fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-")); + const managedUserDir = !resolved.persistent; try { if (nodeModules.length > 0) { @@ -234,4 +312,9 @@ async function runFlowInvocation({ flow, flowFile, msg, options }) { } } -module.exports = { runFlowInvocation, stderrLogHandler, resolveDefaultUserDir }; +module.exports = { + runFlowInvocation, + stderrLogHandler, + resolveContainerDefaultUserDir, + resolveEffectiveUserDir +}; diff --git a/test/e2e/agentic-workflow-dev-env.e2e.test.js b/test/e2e/agentic-workflow-dev-env.e2e.test.js new file mode 100644 index 0000000..3985bfa --- /dev/null +++ b/test/e2e/agentic-workflow-dev-env.e2e.test.js @@ -0,0 +1,149 @@ +"use strict"; + +/** + * Real, non-mocked e2e coverage for issue #33's `/data` auto-probe against + * the actual motivating image, + * ghcr.io/tbrandenburg/agentic-workflow-dev-env, which sets + * `NODE_RED_HOME=/data` and pre-installs `@tbrandenburg/node-red-agents` + * there -- exactly the real-world case `NODE_RED_CLI_DEFAULT_USERDIR` (see + * #31/#32) was reverted for being dead-on-arrival against. + * + * `--docker `'s normal resolution path derives a sandbox image via + * `npm install -g @tbrandenburg/node-red-cli@` (see + * `src/docker-image.js`), which can't yet reflect this worktree's + * unreleased code. Consistent with `test/integration/docker.integration.test.js`'s + * own documented workaround, this builds a throwaway derived image + * (`FROM ghcr.io/tbrandenburg/agentic-workflow-dev-env:latest` + this + * worktree's own `bin`/`src` copied in directly, no npm install) so the + * actual code under test runs against the actual image, and removes it + * again after the suite runs. + * + * Skipped entirely if Docker isn't reachable, or if the real image can't be + * pulled (no network egress to ghcr.io in this environment) -- never + * mocked. + */ + +const assert = require("node:assert/strict"); +const fs = require("node:fs"); +const os = require("node:os"); +const path = require("node:path"); +const { spawnSync, spawn } = require("node:child_process"); +const { before, after, test } = require("node:test"); + +const REPO_ROOT = path.join(__dirname, "..", ".."); +const BASE_IMAGE = "ghcr.io/tbrandenburg/agentic-workflow-dev-env:latest"; +const DERIVED_IMAGE = "node-red-cli-agentic-dev-env-e2e-test:local"; + +function dockerAvailable() { + const result = spawnSync("docker", ["info"], { stdio: ["ignore", "ignore", "ignore"] }); + return !result.error && result.status === 0; +} + +function baseImagePullable() { + const result = spawnSync("docker", ["pull", BASE_IMAGE], { + stdio: ["ignore", "ignore", "ignore"], + timeout: 2 * 60 * 1000 + }); + return !result.error && result.status === 0; +} + +let skip = !dockerAvailable(); +let skipReason = "Docker daemon not reachable in this environment"; +if (!skip && !baseImagePullable()) { + skip = true; + skipReason = `could not pull ${BASE_IMAGE} (no network egress to ghcr.io in this environment)`; +} + +function runCli(args, input) { + return new Promise((resolve, reject) => { + const cliPath = path.join(REPO_ROOT, "bin", "node-red-cli.js"); + const child = spawn(process.execPath, [cliPath, ...args], { env: process.env }); + let stdout = ""; + let stderr = ""; + child.stdout.on("data", (chunk) => (stdout += chunk)); + child.stderr.on("data", (chunk) => (stderr += chunk)); + child.on("error", reject); + child.on("close", (code) => resolve({ code, stdout, stderr })); + child.stdin.end(input); + }); +} + +before(async function () { + if (skip) { + this.skip(skipReason); + return; + } + + const dockerfile = [ + `FROM ${BASE_IMAGE}`, + "USER root", + "WORKDIR /usr/local/lib/node_modules/@tbrandenburg/node-red-cli", + "COPY package.json package-lock.json ./", + "RUN npm ci --omit=dev --no-audit --no-fund", + "COPY bin ./bin", + "COPY src ./src", + 'ENTRYPOINT ["node", "/usr/local/lib/node_modules/@tbrandenburg/node-red-cli/bin/node-red-cli-sandbox-entry.js"]', + "" + ].join("\n"); + + const dockerfileDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-agentic-e2e-")); + const dockerfilePath = path.join(dockerfileDir, "Dockerfile"); + fs.writeFileSync(dockerfilePath, dockerfile); + + const build = spawnSync("docker", ["build", "-t", DERIVED_IMAGE, "-f", dockerfilePath, REPO_ROOT], { + stdio: ["ignore", "pipe", "pipe"], + timeout: 5 * 60 * 1000 + }); + fs.rmSync(dockerfileDir, { recursive: true, force: true }); + + if (build.status !== 0) { + throw new Error( + `failed to build the throwaway agentic-workflow-dev-env-derived test image: ${build.stderr}` + ); + } +}); + +after(() => { + if (skip) return; + spawnSync("docker", ["image", "rm", "-f", DERIVED_IMAGE], { stdio: "ignore" }); +}); + +test( + "e2e: --docker discovers the real @tbrandenburg/node-red-agents module via the /data auto-probe, no flags needed (issue #33)", + { skip }, + async () => { + // The 'agent' node is deployed unwired (alongside a plain ask->return + // link pair) purely to force Node-RED to load + // @tbrandenburg/node-red-agents from userDir -- which only succeeds if + // the /data auto-probe actually picked /data as userDir. Node-RED + // refuses to start *any* flow if *any* referenced node type isn't + // registered (see src/run-envelope.js's waitForFlowsSettled doc + // comment), so a successful ask/return round trip here is proof the + // real module was discovered and loaded -- without needing to actually + // invoke the agent (which would need a real coding-agent API key). + const flow = JSON.stringify([ + { 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" }, + { + id: "unused-agent", + type: "agent", + z: "tab", + name: "unused-agent", + agent: "opencode", + runtime: "direct", + prompt: "payload", + promptType: "msg", + wires: [[], []] + } + ]); + + const { code, stdout, stderr } = await runCli( + ["--flow-json", flow, "ask", "--docker", DERIVED_IMAGE, "--format=json"], + JSON.stringify({ payload: "hi" }) + ); + + assert.equal(code, 0, stderr); + assert.deepEqual(JSON.parse(stdout).payload, "hi"); + } +); diff --git a/test/integration/docker.integration.test.js b/test/integration/docker.integration.test.js index 0357097..85cea12 100644 --- a/test/integration/docker.integration.test.js +++ b/test/integration/docker.integration.test.js @@ -31,10 +31,12 @@ const { buildRunArgs } = require("../../src/docker-run"); const REPO_ROOT = path.join(__dirname, "..", ".."); const TEST_IMAGE = "node-red-cli-sandbox-integration-test:local"; -const DEFAULT_USERDIR_TEST_IMAGE = "node-red-cli-sandbox-default-userdir-test:local"; -const BAKED_USERDIR_PATH = "/opt/preinstalled-userdir"; -const BAKED_MODULE_NAME = "node-red-contrib-cli-issue-31-dummy"; -const BAKED_NODE_TYPE = "cli-issue-31-dummy"; +const DATA_PROBE_TEST_IMAGE = "node-red-cli-sandbox-data-probe-test:local"; +const DATA_PROBE_NEGATIVE_TEST_IMAGE = "node-red-cli-sandbox-data-probe-negative-test:local"; +const DOCKER_USERDIR_TEST_IMAGE = "node-red-cli-sandbox-docker-userdir-test:local"; +const DOCKER_USERDIR_PATH = "/opt/preinstalled-userdir"; +const BAKED_MODULE_NAME = "@test/cli-issue-33-dummy"; +const BAKED_NODE_TYPE = "cli-issue-33-dummy"; function dockerAvailable() { const result = spawnSync("docker", ["info"], { stdio: ["ignore", "ignore", "ignore"] }); @@ -88,10 +90,8 @@ before(async function () { throw new Error(`failed to build the throwaway test image: ${build.stderr}`); } - // Second throwaway image (issue #31): derived from the same base, but also bakes - // a dummy Node-RED node module into a fixed path and sets - // NODE_RED_CLI_DEFAULT_USERDIR to it, simulating a community image that - // pre-installs node packages into its own conventional default userDir. + // Fake Node-RED node package module baked into each of the three + // throwaway images below (issue #33), shared as a single directory tree. const moduleDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-baked-module-")); fs.writeFileSync( path.join(moduleDir, "package.json"), @@ -116,31 +116,78 @@ before(async function () { ].join("\n") ); - const defaultUserDirDockerfile = [ + // Image 1 (issue #33): bakes the module into /data/node_modules/@test/*, + // simulating a community image (like the motivating + // ghcr.io/tbrandenburg/agentic-workflow-dev-env) whose own conventional + // default userDir the /data auto-probe should discover, with no + // --user-dir/--docker-userdir/--node-modules given at all. + const dataProbeDockerfile = [ `FROM ${TEST_IMAGE}`, - `RUN mkdir -p ${BAKED_USERDIR_PATH}/node_modules/${BAKED_MODULE_NAME}`, - `COPY baked-module/ ${BAKED_USERDIR_PATH}/node_modules/${BAKED_MODULE_NAME}/`, - `ENV NODE_RED_CLI_DEFAULT_USERDIR=${BAKED_USERDIR_PATH}`, + `RUN mkdir -p /data/node_modules/${BAKED_MODULE_NAME}`, + `COPY baked-module/ /data/node_modules/${BAKED_MODULE_NAME}/`, "" ].join("\n"); + const dataProbeBuildDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-docker-data-probe-test-")); + fs.cpSync(moduleDir, path.join(dataProbeBuildDir, "baked-module"), { recursive: true }); + fs.writeFileSync(path.join(dataProbeBuildDir, "Dockerfile"), dataProbeDockerfile); - const defaultUserDirBuildDir = fs.mkdtempSync( - path.join(os.tmpdir(), "node-red-cli-docker-default-userdir-test-") + const dataProbeBuild = spawnSync("docker", ["build", "-t", DATA_PROBE_TEST_IMAGE, dataProbeBuildDir], { + stdio: ["ignore", "pipe", "pipe"], + timeout: 5 * 60 * 1000 + }); + fs.rmSync(dataProbeBuildDir, { recursive: true, force: true }); + if (dataProbeBuild.status !== 0) { + throw new Error(`failed to build the throwaway /data auto-probe test image: ${dataProbeBuild.stderr}`); + } + + // Image 2 (issue #33): /data exists but contains no valid Node-RED + // package -- the auto-probe must reject it and fall through to the + // normal ephemeral default instead of misidentifying an unrelated /data. + const dataProbeNegativeDockerfile = [ + `FROM ${TEST_IMAGE}`, + "RUN mkdir -p /data/node_modules/some-unrelated-package", + 'RUN echo \'{"name":"some-unrelated-package"}\' > /data/node_modules/some-unrelated-package/package.json', + "" + ].join("\n"); + const dataProbeNegativeBuildDir = fs.mkdtempSync( + path.join(os.tmpdir(), "node-red-cli-docker-data-probe-negative-test-") ); - fs.cpSync(moduleDir, path.join(defaultUserDirBuildDir, "baked-module"), { recursive: true }); - fs.writeFileSync(path.join(defaultUserDirBuildDir, "Dockerfile"), defaultUserDirDockerfile); - fs.rmSync(moduleDir, { recursive: true, force: true }); + fs.writeFileSync(path.join(dataProbeNegativeBuildDir, "Dockerfile"), dataProbeNegativeDockerfile); - const defaultUserDirBuild = spawnSync( + const dataProbeNegativeBuild = spawnSync( "docker", - ["build", "-t", DEFAULT_USERDIR_TEST_IMAGE, defaultUserDirBuildDir], + ["build", "-t", DATA_PROBE_NEGATIVE_TEST_IMAGE, dataProbeNegativeBuildDir], { stdio: ["ignore", "pipe", "pipe"], timeout: 5 * 60 * 1000 } ); - fs.rmSync(defaultUserDirBuildDir, { recursive: true, force: true }); + fs.rmSync(dataProbeNegativeBuildDir, { recursive: true, force: true }); + if (dataProbeNegativeBuild.status !== 0) { + throw new Error( + `failed to build the throwaway /data auto-probe negative test image: ${dataProbeNegativeBuild.stderr}` + ); + } + + // Image 3 (issue #33): bakes the module into a non-/data path, exercising + // the explicit --docker-userdir override rather than the /data auto-probe. + const dockerUserDirDockerfile = [ + `FROM ${TEST_IMAGE}`, + `RUN mkdir -p ${DOCKER_USERDIR_PATH}/node_modules/${BAKED_MODULE_NAME}`, + `COPY baked-module/ ${DOCKER_USERDIR_PATH}/node_modules/${BAKED_MODULE_NAME}/`, + "" + ].join("\n"); + const dockerUserDirBuildDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-docker-userdir-test-")); + fs.cpSync(moduleDir, path.join(dockerUserDirBuildDir, "baked-module"), { recursive: true }); + fs.writeFileSync(path.join(dockerUserDirBuildDir, "Dockerfile"), dockerUserDirDockerfile); + fs.rmSync(moduleDir, { recursive: true, force: true }); - if (defaultUserDirBuild.status !== 0) { + const dockerUserDirBuild = spawnSync( + "docker", + ["build", "-t", DOCKER_USERDIR_TEST_IMAGE, dockerUserDirBuildDir], + { stdio: ["ignore", "pipe", "pipe"], timeout: 5 * 60 * 1000 } + ); + fs.rmSync(dockerUserDirBuildDir, { recursive: true, force: true }); + if (dockerUserDirBuild.status !== 0) { throw new Error( - `failed to build the throwaway default-userdir test image: ${defaultUserDirBuild.stderr}` + `failed to build the throwaway --docker-userdir test image: ${dockerUserDirBuild.stderr}` ); } }); @@ -148,7 +195,9 @@ before(async function () { after(() => { if (skip) return; spawnSync("docker", ["image", "rm", "-f", TEST_IMAGE], { stdio: "ignore" }); - spawnSync("docker", ["image", "rm", "-f", DEFAULT_USERDIR_TEST_IMAGE], { stdio: "ignore" }); + spawnSync("docker", ["image", "rm", "-f", DATA_PROBE_TEST_IMAGE], { stdio: "ignore" }); + spawnSync("docker", ["image", "rm", "-f", DATA_PROBE_NEGATIVE_TEST_IMAGE], { stdio: "ignore" }); + spawnSync("docker", ["image", "rm", "-f", DOCKER_USERDIR_TEST_IMAGE], { stdio: "ignore" }); }); test( @@ -317,7 +366,52 @@ test("docker integration: --network alone (no --node-modules) enables network ac }); test( - "docker integration: --docker discovers the pre-baked module without --user-dir/--node-modules (issue #31)", + "docker integration: --docker discovers it via the auto-probe, no flags needed (issue #33)", + { skip }, + async () => { + const flow = JSON.stringify([ + { id: "tab", type: "tab", label: "t" }, + { id: "ask", type: "link in", z: "tab", name: "ask", wires: [["dummy"]] }, + { id: "dummy", type: BAKED_NODE_TYPE, z: "tab", name: "d", wires: [["return"]] }, + { id: "return", type: "link out", z: "tab", name: "return", mode: "return" } + ]); + const { code, stdout, stderr } = await runCli( + ["--flow-json", flow, "ask", "--docker", DATA_PROBE_TEST_IMAGE, "--format=json"], + JSON.stringify({ payload: "hi" }) + ); + + assert.equal(code, 0, stderr); + assert.deepEqual(JSON.parse(stdout).payload, "hi"); + } +); + +test( + "docker integration: --docker falls through to the ephemeral default (issue #33)", + { skip }, + async () => { + const flowsPath = path.join(REPO_ROOT, "test", "fixtures", "single-link-in.flows.json"); + const { code, stdout, stderr } = await runCli( + [ + flowsPath, + "calculate", + "--docker", + DATA_PROBE_NEGATIVE_TEST_IMAGE, + "--set", + "x=4", + "--set", + "y=5", + "--format=json" + ], + "" + ); + + assert.equal(code, 0, stderr); + assert.equal(JSON.parse(stdout).payload, 9); + } +); + +test( + "docker integration: --docker-userdir discovers a pre-baked module at a non-/data path (issue #33)", { skip }, async () => { const flow = JSON.stringify([ @@ -327,7 +421,16 @@ test( { id: "return", type: "link out", z: "tab", name: "return", mode: "return" } ]); const { code, stdout, stderr } = await runCli( - ["--flow-json", flow, "ask", "--docker", DEFAULT_USERDIR_TEST_IMAGE, "--format=json"], + [ + "--flow-json", + flow, + "ask", + "--docker", + DOCKER_USERDIR_TEST_IMAGE, + "--docker-userdir", + DOCKER_USERDIR_PATH, + "--format=json" + ], JSON.stringify({ payload: "hi" }) ); @@ -337,10 +440,10 @@ test( ); test( - "docker integration: explicit --user-dir takes precedence over NODE_RED_CLI_DEFAULT_USERDIR (the pre-baked module is not used)", + "docker integration: explicit --user-dir takes precedence over --docker-userdir (the pre-baked module is not used)", { skip }, async () => { - const userDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-docker-default-userdir-precedence-")); + const userDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-docker-userdir-precedence-")); fs.rmSync(userDir, { recursive: true, force: true }); // deterministic never-used path for volume naming const flow = JSON.stringify([ { id: "tab", type: "tab", label: "t" }, @@ -354,7 +457,9 @@ test( flow, "ask", "--docker", - DEFAULT_USERDIR_TEST_IMAGE, + DOCKER_USERDIR_TEST_IMAGE, + "--docker-userdir", + DOCKER_USERDIR_PATH, "--user-dir", userDir, "--format=json" diff --git a/test/integration/run-envelope.integration.test.js b/test/integration/run-envelope.integration.test.js index 770e7cf..1c710de 100644 --- a/test/integration/run-envelope.integration.test.js +++ b/test/integration/run-envelope.integration.test.js @@ -89,62 +89,49 @@ test("integration: runFlowInvocation deploys and calls a flow whose nodes omit e }); /** - * Integration coverage for issue #31: `NODE_RED_CLI_DEFAULT_USERDIR` lets a - * Docker image's own pre-populated default userDir be discovered by - * `runFlowInvocation` when `--user-dir` isn't given. Exercised against the - * real embedded Node-RED runtime since `runFlowInvocation` boots one - * directly; not feasibly unit-testable against a fake runtime. + * Integration coverage for issue #33: `runFlowInvocation`'s `dockerUserDir` + * and `probeContainerDefault` options against the real embedded Node-RED + * runtime (exercised here since `runFlowInvocation` boots one directly). + * The `/data` auto-probe's own validation logic is unit-tested in isolation + * (`test/unit/run-envelope.unit.test.js`); this only confirms + * `runFlowInvocation` actually wires `dockerUserDir` through as a + * persistent userDir, never removed afterward, same as an explicit + * `--user-dir`. */ -const ENV_VAR = "NODE_RED_CLI_DEFAULT_USERDIR"; - -test("integration: runFlowInvocation uses NODE_RED_CLI_DEFAULT_USERDIR as userDir and never removes it", async () => { - const originalEnvValue = process.env[ENV_VAR]; - const defaultUserDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-default-userdir-")); - const markerFile = path.join(defaultUserDir, "marker.txt"); +test("integration: runFlowInvocation uses dockerUserDir as userDir and never removes it", async () => { + const dockerUserDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-docker-userdir-")); + const markerFile = path.join(dockerUserDir, "marker.txt"); fs.writeFileSync(markerFile, "keep-me"); - process.env[ENV_VAR] = defaultUserDir; try { const result = await runFlowInvocation({ flow: SELF_NAMED_LINK_FLOW, msg: { payload: "hi" }, - options: { target: "ask", timeoutMs: 5000, format: "json" } + options: { target: "ask", timeoutMs: 5000, format: "json", dockerUserDir } }); assert.deepEqual(JSON.parse(result.output).payload, "hi"); - assert.equal(fs.existsSync(defaultUserDir), true); + assert.equal(fs.existsSync(dockerUserDir), true); assert.equal(fs.readFileSync(markerFile, "utf8"), "keep-me"); } finally { - if (originalEnvValue === undefined) { - delete process.env[ENV_VAR]; - } else { - process.env[ENV_VAR] = originalEnvValue; - } - fs.rmSync(defaultUserDir, { recursive: true, force: true }); + fs.rmSync(dockerUserDir, { recursive: true, force: true }); } }); -test("integration: explicit userDir takes precedence over NODE_RED_CLI_DEFAULT_USERDIR", async () => { - const originalEnvValue = process.env[ENV_VAR]; - const defaultUserDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-default-userdir-")); +test("integration: explicit userDir takes precedence over dockerUserDir", async () => { + const dockerUserDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-docker-userdir-")); const explicitUserDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-explicit-userdir-")); - process.env[ENV_VAR] = defaultUserDir; try { const result = await runFlowInvocation({ flow: SELF_NAMED_LINK_FLOW, msg: { payload: "hi" }, - options: { target: "ask", timeoutMs: 5000, format: "json", userDir: explicitUserDir } + options: { target: "ask", timeoutMs: 5000, format: "json", userDir: explicitUserDir, dockerUserDir } }); assert.deepEqual(JSON.parse(result.output).payload, "hi"); - // The default userDir must be left untouched -- proof it was never loaded. - assert.deepEqual(fs.readdirSync(defaultUserDir), []); + // The dockerUserDir must be left untouched -- proof it was never loaded. + assert.deepEqual(fs.readdirSync(dockerUserDir), []); } finally { - if (originalEnvValue === undefined) { - delete process.env[ENV_VAR]; - } else { - process.env[ENV_VAR] = originalEnvValue; - } - fs.rmSync(defaultUserDir, { recursive: true, force: true }); + fs.rmSync(dockerUserDir, { recursive: true, force: true }); fs.rmSync(explicitUserDir, { recursive: true, force: true }); } }); diff --git a/test/unit/run-envelope.unit.test.js b/test/unit/run-envelope.unit.test.js index c47b20e..5d39ba3 100644 --- a/test/unit/run-envelope.unit.test.js +++ b/test/unit/run-envelope.unit.test.js @@ -4,84 +4,160 @@ const assert = require("node:assert/strict"); const fs = require("node:fs"); const os = require("node:os"); const path = require("node:path"); -const { test, beforeEach, afterEach } = require("node:test"); -const { resolveDefaultUserDir } = require("../../src/run-envelope"); +const { test } = require("node:test"); +const { resolveContainerDefaultUserDir, resolveEffectiveUserDir } = require("../../src/run-envelope"); -const ENV_VAR = "NODE_RED_CLI_DEFAULT_USERDIR"; -let originalEnvValue; +/** + * Unit coverage for issue #33's `resolveContainerDefaultUserDir` helper -- + * the `/data` auto-probe that lets a Docker image's own pre-populated + * default userDir be discovered without any bespoke env-var convention + * (replacing the reverted `NODE_RED_CLI_DEFAULT_USERDIR`, see #31/#32). + * `baseDir` is parameterized precisely so this is testable against real + * temp directories on disk without touching the real `/data` (which + * generally doesn't exist outside a container anyway). + */ +test("unit: resolveContainerDefaultUserDir returns undefined when baseDir doesn't exist", () => { + const missingDir = path.join(os.tmpdir(), "node-red-cli-data-probe-missing-", String(Date.now())); + assert.equal(resolveContainerDefaultUserDir(missingDir), undefined); +}); -beforeEach(() => { - originalEnvValue = process.env[ENV_VAR]; - delete process.env[ENV_VAR]; +test("unit: resolveContainerDefaultUserDir returns undefined when baseDir has no node_modules", () => { + const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-data-probe-empty-")); + try { + assert.equal(resolveContainerDefaultUserDir(baseDir), undefined); + } finally { + fs.rmSync(baseDir, { recursive: true, force: true }); + } }); -afterEach(() => { - if (originalEnvValue === undefined) { - delete process.env[ENV_VAR]; - } else { - process.env[ENV_VAR] = originalEnvValue; +test("unit: resolveContainerDefaultUserDir returns undefined when node_modules has only unrelated packages (true negative)", () => { + const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-data-probe-unrelated-")); + try { + const pkgDir = path.join(baseDir, "node_modules", "some-unrelated-package"); + fs.mkdirSync(pkgDir, { recursive: true }); + fs.writeFileSync(path.join(pkgDir, "package.json"), JSON.stringify({ name: "some-unrelated-package" })); + assert.equal(resolveContainerDefaultUserDir(baseDir), undefined); + } finally { + fs.rmSync(baseDir, { recursive: true, force: true }); } }); -/** - * Unit coverage for issue #31's `resolveDefaultUserDir` helper -- the - * env-var-driven fallback that lets a Docker image's own pre-populated - * default userDir be discovered by `runFlowInvocation` when `--user-dir` - * isn't given. Full precedence/non-deletion behavior of `runFlowInvocation` - * itself is covered at the integration level (real Node-RED runtime boot - * required), since `runFlowInvocation` isn't feasibly unit-testable against - * a fake runtime without mocking the `node-red` module. - */ -test("unit: resolveDefaultUserDir returns undefined when NODE_RED_CLI_DEFAULT_USERDIR is unset", () => { - assert.equal(resolveDefaultUserDir(), undefined); +test("unit: resolveContainerDefaultUserDir returns baseDir when a direct node_modules child declares 'node-red' (true positive)", () => { + const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-data-probe-direct-")); + try { + const pkgDir = path.join(baseDir, "node_modules", "node-red-contrib-example"); + fs.mkdirSync(pkgDir, { recursive: true }); + fs.writeFileSync( + path.join(pkgDir, "package.json"), + JSON.stringify({ name: "node-red-contrib-example", "node-red": { nodes: { example: "index.js" } } }) + ); + assert.equal(resolveContainerDefaultUserDir(baseDir), baseDir); + } finally { + fs.rmSync(baseDir, { recursive: true, force: true }); + } }); -test("unit: resolveDefaultUserDir returns the path when it exists and is a directory", () => { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-default-userdir-")); +test("unit: resolveContainerDefaultUserDir returns baseDir when a scoped (@scope/*) package declares 'node-red' (true positive)", () => { + const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-data-probe-scoped-")); try { - process.env[ENV_VAR] = dir; - assert.equal(resolveDefaultUserDir(), dir); + const pkgDir = path.join(baseDir, "node_modules", "@example-scope", "example-nodes"); + fs.mkdirSync(pkgDir, { recursive: true }); + fs.writeFileSync( + path.join(pkgDir, "package.json"), + JSON.stringify({ name: "@example-scope/example-nodes", "node-red": { nodes: { example: "index.js" } } }) + ); + assert.equal(resolveContainerDefaultUserDir(baseDir), baseDir); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + fs.rmSync(baseDir, { recursive: true, force: true }); } }); -test("unit: resolveDefaultUserDir falls back to undefined and warns when the path doesn't exist", () => { - const missingPath = path.join(os.tmpdir(), "node-red-cli-does-not-exist-", String(Date.now())); - process.env[ENV_VAR] = missingPath; +test("unit: resolveContainerDefaultUserDir follows symlinked scoped packages (npm frequently symlinks installs)", () => { + const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-data-probe-symlink-")); + try { + const realPkgDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-data-probe-symlink-target-")); + fs.writeFileSync( + path.join(realPkgDir, "package.json"), + JSON.stringify({ name: "@example-scope/example-nodes", "node-red": { nodes: { example: "index.js" } } }) + ); + const scopeDir = path.join(baseDir, "node_modules", "@example-scope"); + fs.mkdirSync(scopeDir, { recursive: true }); + fs.symlinkSync(realPkgDir, path.join(scopeDir, "example-nodes"), "dir"); - const originalConsoleError = console.error; - const warnings = []; - console.error = (msg) => warnings.push(msg); + assert.equal(resolveContainerDefaultUserDir(baseDir), baseDir); + fs.rmSync(realPkgDir, { recursive: true, force: true }); + } finally { + fs.rmSync(baseDir, { recursive: true, force: true }); + } +}); + +/** + * Unit coverage for `resolveEffectiveUserDir`'s full 4-level precedence + * order (see #33): explicit `--user-dir` > explicit `--docker-userdir` > + * the auto-probed `/data` default (only attempted when + * `probeContainerDefault` is set) > the ephemeral fallback (represented + * here as `undefined`, since the ephemeral tmpdir itself is created by the + * caller, `runFlowInvocation`). + */ +test("unit: resolveEffectiveUserDir level 1 -- explicit userDir wins over everything else", () => { + const result = resolveEffectiveUserDir({ + userDir: "/explicit/user-dir", + dockerUserDir: "/docker/user-dir", + probeContainerDefault: true + }); + assert.deepEqual(result, { userDir: "/explicit/user-dir", persistent: true }); +}); + +test("unit: resolveEffectiveUserDir level 2 -- dockerUserDir wins when userDir is absent", () => { + const result = resolveEffectiveUserDir({ dockerUserDir: "/docker/user-dir", probeContainerDefault: true }); + assert.deepEqual(result, { userDir: "/docker/user-dir", persistent: true }); +}); + +test("unit: resolveEffectiveUserDir level 3 -- auto-probed /data wins when userDir/dockerUserDir are both absent", () => { + const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-precedence-probe-")); try { - assert.equal(resolveDefaultUserDir(), undefined); + const pkgDir = path.join(baseDir, "node_modules", "node-red-contrib-example"); + fs.mkdirSync(pkgDir, { recursive: true }); + fs.writeFileSync( + path.join(pkgDir, "package.json"), + JSON.stringify({ name: "node-red-contrib-example", "node-red": {} }) + ); + + const result = resolveEffectiveUserDir({ probeContainerDefault: true, probeBaseDir: baseDir }); + assert.deepEqual(result, { userDir: baseDir, persistent: true }); } finally { - console.error = originalConsoleError; + fs.rmSync(baseDir, { recursive: true, force: true }); } +}); - assert.equal(warnings.length, 1); - assert.match(warnings[0], /NODE_RED_CLI_DEFAULT_USERDIR='.*' is not usable/); - assert.match(warnings[0], /falling back to an ephemeral userDir/); +test("unit: resolveEffectiveUserDir level 3 -- an unusable auto-probe base falls through to level 4 (not persistent)", () => { + const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-precedence-probe-negative-")); + try { + const result = resolveEffectiveUserDir({ probeContainerDefault: true, probeBaseDir: baseDir }); + assert.deepEqual(result, { userDir: undefined, persistent: false }); + } finally { + fs.rmSync(baseDir, { recursive: true, force: true }); + } }); -test("unit: resolveDefaultUserDir falls back to undefined and warns when the path is a file, not a directory", () => { - const filePath = path.join( - fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-default-userdir-file-")), - "not-a-dir" - ); - fs.writeFileSync(filePath, ""); - process.env[ENV_VAR] = filePath; - - const originalConsoleError = console.error; - const warnings = []; - console.error = (msg) => warnings.push(msg); +test("unit: resolveEffectiveUserDir level 3 -- probeContainerDefault false skips the auto-probe even if it would otherwise succeed", () => { + const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), "node-red-cli-precedence-skip-probe-")); try { - assert.equal(resolveDefaultUserDir(), undefined); + const pkgDir = path.join(baseDir, "node_modules", "node-red-contrib-example"); + fs.mkdirSync(pkgDir, { recursive: true }); + fs.writeFileSync( + path.join(pkgDir, "package.json"), + JSON.stringify({ name: "node-red-contrib-example", "node-red": {} }) + ); + + const result = resolveEffectiveUserDir({ probeBaseDir: baseDir }); + assert.deepEqual(result, { userDir: undefined, persistent: false }); } finally { - console.error = originalConsoleError; - fs.rmSync(path.dirname(filePath), { recursive: true, force: true }); + fs.rmSync(baseDir, { recursive: true, force: true }); } +}); - assert.equal(warnings.length, 1); - assert.match(warnings[0], /not a directory/); +test("unit: resolveEffectiveUserDir level 4 -- falls back to undefined/not-persistent when nothing else applies", () => { + const result = resolveEffectiveUserDir({}); + assert.deepEqual(result, { userDir: undefined, persistent: false }); });