Skip to content

Fix system log leak and move web tooling to Node 24 - #478

Merged
matteius merged 1 commit into
mainfrom
fix/system-logs-leak-node-24-babel-8
Aug 3, 2026
Merged

matteius merged 1 commit into
mainfrom
fix/system-logs-leak-node-24-babel-8

Conversation

@matteius

@matteius matteius commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • fix ownership of the cJSON level reference in the system-log tail endpoint so every entry is released with its parent
  • add a LeakSanitizer regression test covering multiple parsed log entries
  • standardize local, CI, devcontainer, and Docker builds on Node 24 (>=24.11.0 <25) with .nvmrc pinned to 24.19.0
  • upgrade the direct Babel toolchain to Babel 8 and replace the Babel-7-only @preact/preset-vite integration with Vite 8 Oxc JSX plus @prefresh/vite
  • retain @vitejs/plugin-legacy and legacy browser bundles

Validation

  • ASAN/LSAN unit suite: 13 passed; no leaks
  • targeted /api/system/logs Playwright test: passed; clean sanitizer teardown
  • full Playwright suite: 168 passed and 2 transient state/timing failures; both passed with --last-failed; no LeakSanitizer finding during teardown
  • web Jest suite: 6 suites and 59 tests passed
  • Vite production build: passed for modern and legacy bundles
  • root and web npm audits: 0 vulnerabilities
  • Node 24 Docker builder stage: passed, including the complete web and native application builds

Known install warning

npm ci emits non-fatal peer warnings from Jest dependency babel-preset-current-node-syntax, whose transitive Babel syntax plugins still declare Babel 7-only peer metadata. babel-jest 30 itself declares Babel 8 support, and the complete Jest suite passes on Babel 8.

Copilot AI review requested due to automatic review settings August 3, 2026 21:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a memory leak in the system log tail API by correcting cJSON reference-node ownership, and modernizes the web/tooling stack by standardizing on Node.js 24 and updating the Vite/Preact build pipeline to avoid Babel-7-only dependencies.

Changes:

  • Fix cJSON “level” reference-node attachment in /api/system/logs tail JSON generation and add a LeakSanitizer regression unit test.
  • Standardize local/CI/devcontainer/Docker builds on Node.js >=24.11.0 <25 (with .nvmrc pinned to 24.19.0).
  • Update the web build tooling to use Vite 8’s JSX handling targeting Preact directly, and replace @preact/preset-vite with @prefresh/vite.

Reviewed changes

Copilot reviewed 15 out of 17 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/web/api_handlers_system_logs_tail.c Fixes cJSON ownership by attaching the created reference node directly to the log entry.
tests/unit/test_api_handlers_system.c Adds a unit test intended to catch regressions in log entry reference-node cleanup under LSAN.
web/vite.config.js Switches from @preact/preset-vite to @prefresh/vite and configures JSX transform to target Preact.
web/package.json Adds Node engine constraints and updates dev toolchain deps (Babel 8, prefresh).
package.json Adds Node engine constraints for the root Playwright/integration test tooling.
package-lock.json Records the root engines.node constraint in the lockfile metadata.
.nvmrc Pins the repository Node.js version to 24.19.0 for consistent dev/CI usage.
.github/workflows/integration-test.yml Uses actions/setup-node with .nvmrc and enables npm caching instead of installing Node via apt.
Dockerfile Installs Node 24 via NodeSource in the builder stage to align Docker builds with the updated toolchain.
.devcontainer/Dockerfile Updates devcontainer to install Node 24 via NodeSource.
README.md Updates documented prerequisites to require Node >=24.11 <25 for web builds.
scripts/README.md Updates web frontend build prerequisites to Node v24.11+ and <25.
docs/TROUBLESHOOTING_WEB_INTERFACE.md Switches troubleshooting steps to use the repo-pinned Node version via nvm.
docs/RELEASE_PROCESS.md Updates release/build process docs to reflect Node 24 and npm ci --ignore-scripts.
web/tests/README.md Updates local test prerequisites to Node 24.11+ (24.x).
web/tests/RUNNING_TESTS_LOCALLY.md Updates local test prerequisites to Node 24.11+ (24.x).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@matteius
matteius merged commit 34b084c into main Aug 3, 2026
4 checks passed
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.

2 participants