Skip to content

fix(workflow-executor): enforce a minimum Node.js version - #1641

Merged
hercemer42 merged 10 commits into
mainfrom
fix/prd-496-enforce-min-node-version
Jun 10, 2026
Merged

fix(workflow-executor): enforce a minimum Node.js version#1641
hercemer42 merged 10 commits into
mainfrom
fix/prd-496-enforce-min-node-version

Conversation

@hercemer42

@hercemer42 hercemer42 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

The workflow-executor did not declare or enforce a minimum Node.js version. On a too-old runtime it crashed with a cryptic low-level error from its dependencies, giving the user no hint that the Node version was the cause.

This adds:

  • A zero-dependency preflight guard (src/check-node-version.ts) that fails fast with a clear message and a non-zero exit code.
  • engines: { "node": ">=20.0.0" } in package.json — install-time signal.
  • cli.ts runs the guard before loading the heavy CLI modules, so koa/@langchain/openai are never evaluated on a runtime too old to load them. Without this ordering the transitive imports crash first and the user never sees the version message. (Typed via erased import type * as so type safety is preserved.)
  • A README note documenting the requirement.

Why Node 20

20 is the real hard dependency floor (@langchain/openai requires >=20, koa >=18) and an active LTS — the lowest version that actually works, so the executor fails fast below it without rejecting versions that would otherwise run. Trivially overridable via MINIMUM_NODE_MAJOR + the engines string; the tests are floor-agnostic.

Tests

  • New unit tests for the guard: pure version comparison (boundaries, v-prefix, major-only), the message wording, the print + exit(1) behaviour, idempotency, and engines/floor consistency.
  • cli.test.ts regression passes.
  • Three it.todo placeholders flag deliberately-deferred items: malformed-version policy (a decision; process.version is always well-formed) and the before-heavy-imports ordering (best verified end-to-end on a real old runtime).

Notes for the reviewer

  • Draft pending the manual functional check on an actual old Node runtime (confirm the message + non-zero exit), per the test plan.
  • Unrelated / pre-existing: a fresh worktree currently shows tsc errors in @forestadmin/ai-proxy / datasource-toolkit (BaseChatModel, Collection.getOne) — these are present on main and clear once the workspace deps are rebuilt; this PR does not touch those packages.

fixes PRD-496

🤖 Generated with Claude Code

Note

Enforce a minimum Node.js version (>=22.12.0) in the workflow-executor CLI

  • Adds a checkNodeVersion guard in check-node-version.ts that compares process.version against the minimum declared in package.json; exits with code 1 and a descriptive stderr message on unsupported runtimes.
  • Invokes the guard at the top of cli.ts before any other module loads; subsequent imports are made dynamic so the check runs first.
  • Declares "engines": { "node": ">=22.12.0" } in package.json so package managers can also warn or reject installs on unsupported runtimes.
  • Behavioral Change: running the CLI on Node.js < 22.12.0 now terminates immediately with exit code 1 instead of proceeding.

Macroscope summarized 8577b19.

@linear-code

linear-code Bot commented Jun 9, 2026

Copy link
Copy Markdown

PRD-496

Comment thread packages/workflow-executor/test/check-node-version.test.ts Outdated
Comment thread packages/workflow-executor/test/check-node-version.test.ts Outdated
Comment thread packages/workflow-executor/src/check-node-version.ts Outdated
Comment thread packages/workflow-executor/src/check-node-version.ts
Comment thread packages/workflow-executor/src/check-node-version.ts Outdated
Comment thread packages/workflow-executor/src/cli.ts Outdated
Comment thread packages/workflow-executor/src/cli.ts Outdated
Comment thread packages/workflow-executor/README.md Outdated
Comment thread packages/workflow-executor/test/check-node-version.test.ts Outdated
Comment thread packages/workflow-executor/test/check-node-version.test.ts
@hercemer42

Copy link
Copy Markdown
Contributor Author

[claude-opus-4-8] Review resolution summary — flagging 3 points for your call before this advances

Addressed in 38c85928: trimmed the cli.ts comment to 2 lines (#4), reduced the README note to one line (#6), broadened the boundary test to 20.0.0 / 20.0.1 / 20.1.0 (#8), and fixed the misleading test-header comment behind #7. No-race clarification (#1) and the why-defer-the-requires explanation (#5) are in-thread.

Three points I'm pushing back on rather than changing — paused for your decision:

  1. Derive the floor from package.json (fix(interfaces): make schema fields optional and fix typos #3). Doing it correctly needs the semver lib to parse the engines range, which breaks this module's dependency-free constraint (it must load before the heavy imports); a regex is fragile. The single duplicated integer is already cross-checked against engines by a unit test that fails CI on drift. I'll switch to reading package.json if you'd rather have the single source despite the parse — your call.

  2. Unit-test parseMajor directly (feat: implement base interfaces #2). It's private and already covered through isSupportedNodeVersion; testing it directly would go against test-only-public-functions. Would promote it to public if you want a dedicated test.

  3. Use MINIMUM_NODE_MAJOR instead of literal 20 in the pure-function tests (fix: tests were not compiled #7). The literals are intentional fixtures kept floor-agnostic; using the constant would make those tests brittle to a future floor change. Only the default/engines tests are tied to the constant.

Happy to apply any of these if you'd prefer — none are blockers, just judgment calls I wanted your eyes on.

@hercemer42

Copy link
Copy Markdown
Contributor Author

[claude-opus-4-8] Resolution update — the three flagged points are now closed:

Floor behaviour is unchanged (Node 20). 13 tests pass; ESLint + Prettier clean on the changed files. Nothing left open from this round.

Comment thread packages/workflow-executor/src/check-node-version.ts Outdated
Comment thread packages/workflow-executor/src/check-node-version.ts
Comment thread packages/workflow-executor/README.md Outdated
Comment thread packages/workflow-executor/src/check-node-version.ts Outdated
Comment thread packages/workflow-executor/src/check-node-version.ts Outdated
Comment thread packages/workflow-executor/test/check-node-version.test.ts
Comment thread packages/workflow-executor/src/check-node-version.ts Outdated
hercemer42 and others added 8 commits June 10, 2026 11:37
On an unsupported Node.js runtime the executor crashed with a cryptic
low-level error from its dependencies, with no hint that the Node
version was the cause. A preflight guard now fails fast with a clear
message before the heavy imports (koa, @langchain/openai) load, and the
engines field declares the requirement. Minimum is Node 20, the floor
required by @langchain/openai.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Trim the test header to the essential rationale and remove the it.todo
placeholders: malformed input is unreachable since process.version is
always well-formed, and the import-ordering invariant is covered by
manual verification on an old runtime.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address review feedback on the node-version guard: shorten the cli.ts
and README notes, clarify the test header on fixture-vs-floor usage, and
assert minor/patch versions at the floor major.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Read MINIMUM_NODE_MAJOR from the package.json engines range instead of
hardcoding it, so the runtime guard and the install-time engines
declaration share one source and cannot drift. Tests reference the
derived constant rather than literal majors.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the deferred require() + eslint-disable in the bin with typed
`await import()`, which compiles to the same deferred require under
CommonJS but needs no disable and carries the module types. Trim the
guard module comment, and drop the README Node-version note now that
package.json engines is the single source of truth.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Use the single regex-based parseMajor for both the engines range and
the runtime version, removing the duplicate major-parsing logic.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bump engines.node from >=20.0.0 to >=22.12.0; MINIMUM_NODE_MAJOR
derives to 22 accordingly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Compare the full major.minor.patch against engines.node (>=22.12.0)
instead of the major only, so e.g. Node 22.11 is correctly rejected.
MINIMUM_NODE_MAJOR becomes MINIMUM_NODE_VERSION.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the hardcoded floor assertion (toBe("22.12.0")) with a shape
check so it no longer needs editing on every version bump, and drop a
redundant comment on parseVersion.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@hercemer42
hercemer42 force-pushed the fix/prd-496-enforce-min-node-version branch from 3d27771 to c4f7173 Compare June 10, 2026 09:37
@qltysh

qltysh Bot commented Jun 10, 2026

Copy link
Copy Markdown

Qlty


Coverage Impact

⬆️ Merging this pull request will increase total coverage on main by 0.05%.

Modified Files with Diff Coverage (1)

RatingFile% DiffUncovered Line #s
New Coverage rating: A
packages/workflow-executor/src/check-node-version.ts100.0%
Total100.0%
🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

qlty diff coverage flagged the changed lines. Mark the CLI entrypoint
with /* istanbul ignore file */ (side effects, not unit-testable; its
guard and run wiring are unit-tested in check-node-version and cli-core)
and add tests for the default printError/exit path and unparseable
input, taking check-node-version.ts to 100% line coverage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@hercemer42
hercemer42 force-pushed the fix/prd-496-enforce-min-node-version branch from beed3fb to 8577b19 Compare June 10, 2026 10:02
@hercemer42
hercemer42 merged commit 23c594a into main Jun 10, 2026
30 checks passed
@hercemer42
hercemer42 deleted the fix/prd-496-enforce-min-node-version branch June 10, 2026 12:03
forest-bot added a commit that referenced this pull request Jun 10, 2026
## @forestadmin/workflow-executor [1.0.1](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/workflow-executor@1.0.0...@forestadmin/workflow-executor@1.0.1) (2026-06-10)

### Bug Fixes

* **workflow-executor:** enforce a minimum Node.js version ([#1641](#1641)) ([23c594a](23c594a))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants