Repository navigation
fix(MESHAGEN-004-2): CU-86akq5g67 9 review findings across 6 files - #174
flamingo[bot] wants to merge 6 commits into
Conversation
![flamingo[bot]](https://avatars.githubusercontent.com/in/3426709?s=60&v=4){kind=small}
| * limitations under the License. | ||
| */ | ||
|
|
||
| /* |
There was a problem hiding this comment.
🦩 🔴 mac_events.h is missing the required Apache 2.0 / Intel copyright header
Added the standard Apache License 2.0 header block (copyright notice, license grant text, "AS IS" disclaimer) at the very top of meshcore/KVM/MacOS/mac_events.h, before the pre-existing "Created on / Author: Intel" comment, which was left intact. Only mac_events.h was in scope for this task (mac_events.c is a separate file not provided for editing here, so it could not be modified in this response).
🤖 Prompt for AI agents
In meshcore/KVM/MacOS/mac_events.h around line 1, review and complete this code-review fix: mac_events.h is missing the required Apache 2.0 / Intel copyright header.
What the draft fix changed: Added the standard Apache License 2.0 header block (copyright notice, license grant text, "AS IS" disclaimer) at the very top of meshcore/KVM/MacOS/mac_events.h, before the pre-existing "Created on / Author: Intel" comment, which was left intact. Only mac_events.h was in scope for this task (mac_events.c is a separate file not provided for editing here, so it could not be modified in this response).
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟡 85 medium — react 👍/👎 to teach the reviewer
| limitations under the License. | ||
| */ | ||
|
|
||
| #ifndef MAC_LOGGING_UTILS_H |
There was a problem hiding this comment.
🦩 🔴 mac_logging_utils.h is missing the required Apache 2.0 license header
Added the Apache 2.0 license header block with Intel Corporation copyright at the top of meshcore/MacOS/mac_logging_utils.h, before the #ifndef MAC_LOGGING_UTILS_H include guard, exactly as specified in the suggested fix. No other content was changed.
🤖 Prompt for AI agents
In meshcore/MacOS/mac_logging_utils.h around line 1, review and complete this code-review fix: mac_logging_utils.h is missing the required Apache 2.0 license header.
What the draft fix changed: Added the Apache 2.0 license header block with Intel Corporation copyright at the top of meshcore/MacOS/mac_logging_utils.h, before the `#ifndef MAC_LOGGING_UTILS_H` include guard, exactly as specified in the suggested fix. No other content was changed.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer
| limitations under the License. | ||
| */ | ||
|
|
||
| #ifndef MAC_PERMISSIONS_WINDOW_H |
There was a problem hiding this comment.
🦩 🔴 mac_permissions_window.h is missing the required Apache 2.0 license header
Added the required Apache 2.0 license header block (with the Intel Corporation copyright line) at the very top of meshcore/MacOS/TCC_UI/mac_permissions_window.h, before the #ifndef MAC_PERMISSIONS_WINDOW_H include guard, matching the format used in sibling headers.
🤖 Prompt for AI agents
In meshcore/MacOS/TCC_UI/mac_permissions_window.h around line 1, review and complete this code-review fix: mac_permissions_window.h is missing the required Apache 2.0 license header.
What the draft fix changed: Added the required Apache 2.0 license header block (with the Intel Corporation copyright line) at the very top of meshcore/MacOS/TCC_UI/mac_permissions_window.h, before the `#ifndef MAC_PERMISSIONS_WINDOW_H` include guard, matching the format used in sibling headers.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer
| limitations under the License. | ||
| */ | ||
|
|
||
| function getUsers() |
There was a problem hiding this comment.
🦩 🟠 kvm-helper.js missing Apache 2.0 license header
Added the standard Apache 2.0 license header block with Intel copyright at the very top of modules/kvm-helper.js, before the function getUsers() declaration, matching the format specified in the finding and used in modules/logger.js.
🤖 Prompt for AI agents
In modules/kvm-helper.js around line 3, review and complete this code-review fix: kvm-helper.js missing Apache 2.0 license header.
What the draft fix changed: Added the standard Apache 2.0 license header block with Intel copyright at the very top of modules/kvm-helper.js, before the `function getUsers()` declaration, matching the format specified in the finding and used in modules/logger.js.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer
| @@ -243,7 +257,7 @@ if (process.platform == 'linux') | |||
| module.exports = | |||
There was a problem hiding this comment.
🦩 🟠 hasVirtualSessionSupport exported as a boolean value instead of a callable function
Changed hasVirtualSessionSupport: hasVirtualSessionSupport(), in the linux-branch module.exports object (bottom of file) to hasVirtualSessionSupport: hasVirtualSessionSupport,, exporting the function reference instead of its invoked boolean result, consistent with the other function-reference exports like createVirtualSession, users, and loginUids. Note: the non-linux branch still exports a plain boolean false for the same key, so callers that now call hasVirtualSessionSupport() on linux must still handle the non-linux case where it's not callable — this asymmetry was flagged but out of scope to change since the finding only addressed the linux branch's broken function-reference pattern; a complete fix would also normalize the non-linux branch to function () { return false; }, but that would be an additional behavioral change beyond what's shown as the suggested fix.
🤖 Prompt for AI agents
In modules/kvm-helper.js around line 243, review and complete this code-review fix: hasVirtualSessionSupport exported as a boolean value instead of a callable function.
What the draft fix changed: Changed `hasVirtualSessionSupport: hasVirtualSessionSupport(),` in the linux-branch module.exports object (bottom of file) to `hasVirtualSessionSupport: hasVirtualSessionSupport,`, exporting the function reference instead of its invoked boolean result, consistent with the other function-reference exports like `createVirtualSession`, `users`, and `loginUids`. Note: the non-linux branch still exports a plain boolean `false` for the same key, so callers that now call `hasVirtualSessionSupport()` on linux must still handle the non-linux case where it's not callable — this asymmetry was flagged but out of scope to change since the finding only addressed the linux branch's broken function-reference pattern; a complete fix would also normalize the non-linux branch to `function () { return false; }`, but that would be an additional behavioral change beyond what's shown as the suggested fix.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟡 70 medium — react 👍/👎 to teach the reviewer
| @@ -108,7 +122,7 @@ if (process.platform == 'linux') | |||
| { | |||
| var lids = module.exports.allowed | |||
| var uu = {}; | |||
There was a problem hiding this comment.
🦩 🔵 loginUids() uses undeclared global i as loop variable (missing var)
Added var to for (i in lids) inside the loginUids() function, changing it to for (var i in lids), exactly as specified in the suggested fix, preventing the implicit global i.
🤖 Prompt for AI agents
In modules/kvm-helper.js around line 110, review and complete this code-review fix: loginUids() uses undeclared global `i` as loop variable (missing var).
What the draft fix changed: Added `var` to `for (i in lids)` inside the `loginUids()` function, changing it to `for (var i in lids)`, exactly as specified in the suggested fix, preventing the implicit global `i`.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer
| var res, i, uu = {}; | ||
| require('user-sessions').Current(function (u) { res = u; }); | ||
|
|
||
| for (i in res) |
There was a problem hiding this comment.
🦩 🔵 getUsers() loop variable i declared without var, leaking to global scope
Added var to both for (i in res) and for (i in spawnable) loops inside getUsers(), changing them to for (var i in res) and for (var i in spawnable) respectively, exactly as specified in the suggested fix, preventing the implicit global i leak.
🤖 Prompt for AI agents
In modules/kvm-helper.js around line 8, review and complete this code-review fix: getUsers() loop variable `i` declared without var, leaking to global scope.
What the draft fix changed: Added `var` to both `for (i in res)` and `for (i in spawnable)` loops inside `getUsers()`, changing them to `for (var i in res)` and `for (var i in spawnable)` respectively, exactly as specified in the suggested fix, preventing the implicit global `i` leak.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer
| limitations under the License. | ||
| */ | ||
|
|
||
| #ifndef MAC_PLIST_UTILS_H |
There was a problem hiding this comment.
🦩 🟠 mac_plist_utils.h missing Apache 2.0 license header
Added the Apache 2.0 license header block with the Intel Corporation copyright line at the very top of meshcore/MacOS/mac_plist_utils.h, before the #ifndef MAC_PLIST_UTILS_H include guard, matching the suggested fix text exactly.
🤖 Prompt for AI agents
In meshcore/MacOS/mac_plist_utils.h around line 1, review and complete this code-review fix: mac_plist_utils.h missing Apache 2.0 license header.
What the draft fix changed: Added the Apache 2.0 license header block with the Intel Corporation copyright line at the very top of meshcore/MacOS/mac_plist_utils.h, before the `#ifndef MAC_PLIST_UTILS_H` include guard, matching the suggested fix text exactly.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| // stdafx.h : include file for standard system include files, |
There was a problem hiding this comment.
🦩 🔵 meshreset/stdafx.h missing required Apache 2.0 header
Added an Apache License 2.0 header block with Intel Corporation copyright notice at the top of meshreset/stdafx.h, above the existing Visual Studio template comment, matching the pattern expected for files under meshreset/**. The exact copyright year range and holder text are inferred since no project-specific template was provided; a reviewer should confirm the copyright year(s) and exact Intel entity name against other headers already present elsewhere in the repository (e.g. in sibling meshreset files not shown here) before merging.
🤖 Prompt for AI agents
In meshreset/stdafx.h around line 1, review and complete this code-review fix: meshreset/stdafx.h missing required Apache 2.0 header.
What the draft fix changed: Added an Apache License 2.0 header block with Intel Corporation copyright notice at the top of meshreset/stdafx.h, above the existing Visual Studio template comment, matching the pattern expected for files under meshreset/**. The exact copyright year range and holder text are inferred since no project-specific template was provided; a reviewer should confirm the copyright year(s) and exact Intel entity name against other headers already present elsewhere in the repository (e.g. in sibling meshreset files not shown here) before merging.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟡 60 medium — react 👍/👎 to teach the reviewer
flamingo
Bot
changed the title
Closes 9 review findings across 6 files.
Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.
meshcore/KVM/MacOS/mac_events.h:1meshcore/MacOS/mac_logging_utils.h:1meshcore/MacOS/TCC_UI/mac_permissions_window.h:1modules/kvm-helper.js:3modules/kvm-helper.js:243ias loop variable (missing var)modules/kvm-helper.js:110ideclared without var, leaking to global scopemodules/kvm-helper.js:8meshcore/MacOS/mac_plist_utils.h:1meshreset/stdafx.h:1What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.
Run: https://product-hub.flamingo.so/admin/code-review
Run id:
b0044794-23ba-44ff-8104-85f171bb5cdaMerging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.
ClickUp task: CU-86akq5g67 MeshAgent review findings sweep (12 PRs)