Update CLI implementation with modern tooling and conventions by relu91 · Pull Request #1456 · eclipse-thingweb/node-wot

relu91 · 2025-11-28T15:15:46Z

I know that this PR will be long to review, but it has been sitting on my pc for a while, and it is time to push the changes. The primary goal was to get rid of the VM2 module, which is now deprecated and audited as a critical security risk. I used the opportunity to completely revise the implementation and make it:

Hopefully easier to maintain
More useful and self-explanatory

In summary, this PR delivers a significant refactor and several quality-of-life improvements to the cli module, primarily focusing on simplifying configuration, execution, and improving code organization.

Key Changes and Improvements

Configuration Unification: Environment variables are now consistently treated as configuration parameters, simplifying how settings are managed and loaded for the CLI execution.
- ⚠️ Breaking Change: Now you should prefix every env with NODE_WOT_ and the cli automatically map that variable to a property in the configuration schema (e.g., WOT_SERVIENT_SERVIENT_CLIENTONLY -> servient.clientOnly)
Execution Simplification: Simplified the CLI script execution flow; now we simply require the files, avoiding dependence from virtual machines or complicated compilation steps. Virtualization can always be achieved with other means.
- ⚠️ Breaking Change: This drops support for running remote scripts from the Default Servient Thing, a feature that was rarely used and controversial due to security concerns.
Major Refactoring and Testing:
- Completed a major refactor of configuration building, configuration sync with the JSON schema, and logging organization.
- Improved test coverage for the new configuration and parsing functionalities.
MQTT Fix : Allows for optional URI in the MQTT Server configuration.
Audit fix: Now, when you install node-wot, you should not see any audit warnings.

Open Point for Discussion

Given the extensive changes and the focus on the primary user experience:

Should we rename the CLI tool? The current name, wot-servient, is potentially surprising for regular users as it differs from the package name. I propose renaming it to node-wot to align with the project name and make it feel less surprising.

cli.ts log now enables by default error and warning messages.

As a result the responsibility of managing the initial configuration for logging has been moved outside the DefaultServient.

…son schema

WARNING: Drops support for running remote scripts from the Default Servient Thing. This feature was rarely used and controversial due to security implications. Users wanting to run remote scripts can easily implement this feature in their own Servient by extending the CLI Servient.

now envs are treated as config parameters

relu91 · 2025-12-01T14:52:02Z

There is a small problem in the CLI tests due to some file shared across different unit tests. I'll fix them asap. They should not require changes to the business logic of the new CLI.

packages/binding-mqtt/src/mqtt-broker-server.ts

packages/cli/src/parsers/config-file-parser.ts

packages/cli/src/parsers/ip-parser.ts

danielpeintner

Note: I did not fully check nor run it locally, but I would like to make some comments:

THANK YOU for the effort. I think it is very helpful to get rid of VM2
I am surprised that no file has been removed (can it be?)
some more comments, see below

packages/cli/.gitignore

packages/cli/package.json

egekorkan · 2026-01-07T08:02:06Z

Should we rename the CLI tool? The current name, wot-servient, is potentially surprising for regular users as it differs from the package name. I propose renaming it to node-wot to align with the project name and make it feel less surprising.

I like this :) I could not review the rest of the PR yet...

danielpeintner · 2026-01-07T10:00:12Z

Should we rename the CLI tool? The current name, wot-servient, is potentially surprising for regular users as it differs from the package name. I propose renaming it to node-wot to align with the project name and make it feel less surprising.

I like this :) I could not review the rest of the PR yet...

I am fine with changing it. IF WE DO IT, we should publish first a node-wot version and increase the major version to make it clear it is a breaking change.

Co-authored-by: danielpeintner <daniel.peintner@gmail.com>

relu91 · 2026-01-07T15:41:08Z

I am fine with changing it. IF WE DO IT, we should publish first a node-wot version and increase the major version to make it clear it is a breaking change.

Given that we already have two other breaking changes in this PR, it makes sense to target a major version increase anyway. If we can discuss this in the next committer meeting.

danielpeintner

Minor issue

packages/binding-mqtt/README.md

danielpeintner · 2026-01-12T09:46:47Z

@relu91 mentioned on Telegram that vm2 efforts revived
patriksimek/vm2#533 (comment)

Honestly, I don't know which path to go is best... in any case.. even if we would stick to vm2 we should take over some changes Cristiano did in this PR...

relu91 · 2026-01-12T15:44:17Z

As stated offline, I think we can leave vm2 for now. We can re-introduce it if we get real feedback from our users to have isolation between wot scripts and the current Node.js project. So far, the usage has been limited.

danielpeintner · 2026-02-03T16:23:20Z

@relu91 I think we can finally move on with this PR ... 🎉
Unfortunately, there are some conflicts that need to be resolved.

relu91 · 2026-02-18T17:04:42Z

@danielpeintner sadly, I had to re-generate package lock :/ Let me know if you are ok to merge it with these unrelated changes, or if I should try to rebase the whole history from main. That's probably why CI is failing due to coap?

danielpeintner · 2026-02-19T12:47:06Z

Either way is fine for me. We should just manage to get a clean CI run with the right dependencies (versions)

codecov · 2026-02-27T16:04:44Z

Codecov Report

❌ Patch coverage is 93.70079% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.16%. Comparing base (2fd12dd) to head (eb18a6a).
⚠️ Report is 35 commits behind head on master.

Files with missing lines	Patch %	Lines
packages/binding-mqtt/src/mqtt-broker-server.ts	25.00%	6 Missing ⚠️
packages/cli/src/configuration.ts	95.45%	6 Missing ⚠️
packages/cli/src/executor.ts	90.19%	5 Missing ⚠️
packages/cli/src/parsers/config-file-parser.ts	90.32%	3 Missing ⚠️
packages/cli/src/utils/load-env-variables.ts	91.17%	3 Missing ⚠️
packages/cli/src/parsers/config-params-parser.ts	97.56%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1456      +/-   ##
==========================================
+ Coverage   77.99%   78.16%   +0.17%     
==========================================
  Files          86       91       +5     
  Lines       15904    15811      -93     
  Branches     1509     1532      +23     
==========================================
- Hits        12404    12359      -45     
+ Misses       3479     3429      -50     
- Partials       21       23       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

danielpeintner

Minor comments

packages/cli/src/utils/load-env-variables.ts

packages/cli/src/cli.ts

packages/binding-mqtt/README.md

relu91 · 2026-03-03T11:31:30Z

Thank you @danielpeintner ! I think all your comments make sense. It was an open point for discussion, and I forgot to properly align all the naming conventions. Since we settled for renaming wot-servient to simply node-wot, I would simply stick with node-wot everywhere with no servient postfix, okay?

…to node-wot

relu91 · 2026-03-03T11:50:06Z

Done, notice that I removed packages/cli/wot-servient.conf.json.md as it could go out of sync easly. Now the source of truth is the JSON schema, we can reintroduce a reference file once we tackle the problem of good documentation for node-wot.

danielpeintner

Thank you very much @relu91 !!🎉
Looks good to me!

@egekorkan @JKRhb others
do we want to take a final quick look in tomorrows committer call and merge?

egekorkan

@relu91 could not get the linking to work to make node-wot available as a command. npm run link does not work and the instructions are to use the npm package but not what developer should do. bin folder is generated under cli so I can of course do an alias

relu91 added 12 commits November 28, 2025 11:15

refactor(cli): drop vm2 and use NodeJS vm module

e78447b

refactor(cli/default-serivent): use async await in run method

abb813c

refactor(cli): better code organization and minor improvements logging

ee16e04

cli.ts log now enables by default error and warning messages.

refactor(cli): organize logging configuration

e493e36

As a result the responsibility of managing the initial configuration for logging has been moved outside the DefaultServient.

refactor(cli/parsers/config-file-parser): better error messages

6f5953c

refactor(cli): improve configuration building and keep it sync with j…

861bcad

…son schema

fix(mqtt): allow optional uri in MQTT Server configuration

7bb51f3

feat(cli): unify configuration parameters

7269236

now envs are treated as config parameters

chore: remove testing files

1f1d88e

test(cli): cover new functionalities with basic tests

28c8a6f

chore: fix critical audit warnings

93cd428

relu91 requested review from danielpeintner, egekorkan and sebastiankb as code owners November 28, 2025 15:15

relu91 added 2 commits November 28, 2025 16:37

chore: fix eslint warnings

f116cbc

style: run format

5718765

relu91 mentioned this pull request Nov 29, 2025

refactor: eslint no-process-exit, turn off warning in CLI #1433

Open

danielpeintner reviewed Dec 2, 2025

View reviewed changes

packages/binding-mqtt/src/mqtt-broker-server.ts Outdated Show resolved Hide resolved

danielpeintner reviewed Dec 2, 2025

View reviewed changes

packages/cli/src/parsers/config-file-parser.ts Outdated Show resolved Hide resolved

danielpeintner reviewed Dec 2, 2025

View reviewed changes

packages/cli/src/parsers/ip-parser.ts Outdated Show resolved Hide resolved

danielpeintner reviewed Dec 2, 2025

View reviewed changes

packages/cli/.gitignore Show resolved Hide resolved

packages/cli/package.json Outdated Show resolved Hide resolved

test(cli): use tmp library for script files

e5ddb9b

refactor(cli): basic error messages improvments

ca12fe4

Co-authored-by: danielpeintner <daniel.peintner@gmail.com>

relu91 added the Discuss in Committer Meeting Labels issues and PRs that should be discussed in our regular Thingweb Committer Meeting. label Jan 7, 2026

chore(cli): rename import-json.js to import-json-to-ts.js

61768ae

relu91 added 5 commits January 9, 2026 12:11

chore: remove .env file

341cef1

chore(cli): update license date

24dcdf7

refactor(cli): move loglevel to log.level

d823470

test(cli): remove temporaney resource folder

c5a7a51

feat(cli): rename cli commad from wot-servient to node-wot

39877a7

relu91 removed the Discuss in Committer Meeting Labels issues and PRs that should be discussed in our regular Thingweb Committer Meeting. label Jan 9, 2026

danielpeintner reviewed Jan 12, 2026

View reviewed changes

packages/binding-mqtt/README.md Outdated Show resolved Hide resolved

docs(binding-mqtt): fix typo

afc3ccd

relu91 mentioned this pull request Jan 12, 2026

chore(deps): bump vm2 from 3.9.18 to 3.10.0 #1460

Merged

This was referenced Jan 27, 2026

chore(deps): bump vm2 from 3.10.0 to 3.10.2 in /packages/cli #1467

Closed

Updates for node-wot 1.x.x #1473

Open

Merge branch 'master' into cli-enachements

3440b23

relu91 added 2 commits February 27, 2026 16:40

Merge branch 'master' into cli-enachements

8771d96

chore: sync package-lock with master

eb18a6a

relu91 force-pushed the cli-enachements branch from 0c84b23 to eb18a6a Compare February 27, 2026 16:00

danielpeintner approved these changes Mar 2, 2026

View reviewed changes

packages/cli/src/utils/load-env-variables.ts Outdated Show resolved Hide resolved

packages/cli/src/cli.ts Outdated Show resolved Hide resolved

danielpeintner reviewed Mar 3, 2026

View reviewed changes

packages/binding-mqtt/README.md Show resolved Hide resolved

refactor: completely remove references to wot-servient and rename it …

f63f2f7

…to node-wot

relu91 requested a review from mkovatsc as a code owner March 3, 2026 11:48

relu91 requested a review from danielpeintner March 3, 2026 11:50

danielpeintner approved these changes Mar 5, 2026

View reviewed changes

egekorkan reviewed Mar 6, 2026

View reviewed changes

Conversation

relu91 commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Key Changes and Improvements

Open Point for Discussion

Uh oh!

relu91 commented Dec 1, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

danielpeintner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

egekorkan commented Jan 7, 2026

Uh oh!

danielpeintner commented Jan 7, 2026

Uh oh!

relu91 commented Jan 7, 2026

Uh oh!

danielpeintner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

danielpeintner commented Jan 12, 2026

Uh oh!

relu91 commented Jan 12, 2026

Uh oh!

danielpeintner commented Feb 3, 2026

Uh oh!

relu91 commented Feb 18, 2026

Uh oh!

danielpeintner commented Feb 19, 2026

Uh oh!

codecov bot commented Feb 27, 2026

Codecov Report

Uh oh!

danielpeintner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

relu91 commented Mar 3, 2026

Uh oh!

relu91 commented Mar 3, 2026

Uh oh!

danielpeintner left a comment

Choose a reason for hiding this comment

Uh oh!

egekorkan left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

relu91 commented Nov 28, 2025 •

edited

Loading