Skip to content

Conversation

@iotux
Copy link
Contributor

@iotux iotux commented Dec 17, 2025

ℹ️ To keep reviews fast and effective, please make sure you’ve read our pull request guidelines

📝 Summary of changes done and why they are done

Refactored the sendInfo socket handler to group system-related information into a dedicated runtime object.

Changes:

  • Added a runtime object to the root of the info object.
  • Added new extendable fields to runtime: arch, nodeVersion, uptime, and memory (RSS and Heap).
  • Refactored the variable declarations into a single info object for better readability.

Reasoning:

  • Maintainability: Prevents "namespace pollution" in the root info object as we add more metrics.
  • Clarity: Uses the key runtime instead of process to avoid cognitive confusion/naming collisions with the global Node.js process variable.
  • Observability: Provides more detailed server health metrics (memory/uptime) for future dashboard improvements.

📋 Related issues

📄 Checklist

Please follow this checklist to avoid unnecessary back and forth (click to expand)
  • ⚠️ If there are Breaking change (a fix or feature that alters existing functionality in a way that could cause issues) I have called them out
  • 🔍 My code adheres to the style guidelines of this project.
  • 🦿 I have indicated where (if any) I used an LLM for the contributions
  • ✅ I ran ESLint and other code linters for modified files.
  • ⚠️ My changes generate no new warnings.
  • 🛠️ I have reviewed and tested my code.
  • 📝 I have commented my code, especially in hard-to-understand areas (e.g., using JSDoc for methods).
  • 🤖 My code needed automated testing. I have added them (this is an optional task).
  • 📄 Documentation updates are included (if applicable).
  • 🔒 I have considered potential security impacts and mitigated risks.
  • 🧰 Dependency updates are listed and explained.

@iotux iotux force-pushed the feature/platform-meta branch 4 times, most recently from 49ff541 to 7451f66 Compare December 17, 2025 06:47
@iotux
Copy link
Contributor Author

iotux commented Dec 17, 2025

Instead of adding one 'platform' variable to the 'info' object, I decided to add this as an object, where 'platform' is an object. Doing that, opens for future extensions without the risk of cluttering the 'info' object.

Feel free to add or delete elements as you see fit.

@iotux iotux closed this Dec 17, 2025
@iotux iotux reopened this Dec 17, 2025
@CommanderStorm
Copy link
Collaborator

Runtime and arch make sense to me.
For what do you want to use the rest?

@iotux
Copy link
Contributor Author

iotux commented Dec 17, 2025

Runtime and arch make sense to me. For what do you want to use the rest?

I initially set it up that way as examples to show the possibilities, and therefore I commented 'Feel free to add or delete elements as you see fit'. My point was that making this an object instead of a single variable, makes it more open to future enhancements without cluttering the 'info' object.

I will keep 'arch' and 'platform' (linux/win32/whatever).

I am struggling with the Github test environment failing. I have pushed and revoked several times, and I cannot see any apparent reason for the failures. It has failed different places each time, but the 'e2e-tesst' is stubborn. From previous PRs, I have experienced several rounds of test failures before success, even if the code is the same.

@CommanderStorm CommanderStorm changed the title Feature/platform meta feat: add platform/arch information to sendInfo Dec 17, 2025
Copy link
Collaborator

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for this enhancement

@CommanderStorm CommanderStorm marked this pull request as ready for review December 17, 2025 10:56
@CommanderStorm CommanderStorm enabled auto-merge (squash) December 17, 2025 10:57
@iotux iotux closed this Dec 17, 2025
auto-merge was automatically disabled December 17, 2025 11:08

Pull request was closed

@iotux iotux reopened this Dec 17, 2025
@CommanderStorm CommanderStorm enabled auto-merge (squash) December 17, 2025 11:09
@CommanderStorm
Copy link
Collaborator

CommanderStorm commented Dec 17, 2025

why are you closing/reopening your PRs all the time?
This is very noisy for me.

If e2e-tesst flakes but this is not an actual issue, I can rerun it.

@CommanderStorm
Copy link
Collaborator

we should likely still fix the testcase to run reliably though.

@iotux
Copy link
Contributor Author

iotux commented Dec 17, 2025

why are you closing/reopening your PRs all the time? This is very noisy for me.

I haven't found any other way to restart the tests

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.

Expose Host Platform (OS) to Frontend for Adaptive UI and Local Monitoring

2 participants