Skip to content

fix(MESHAGEN-004-2): CU-86akhf8u3 14 review findings across 9 files #123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
flamingo wants to merge 9 commits into
base: master
Choose a base branch
from
+321 −98
Draft

fix(MESHAGEN-004-2): CU-86akhf8u3 14 review findings across 9 files #123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
521cc95
fix(MESHAGEN-004-2): 14 review findings across 9 files
flamingo[bot] Sep 14, 2026
938211d
fix(MESHAGEN-004-2): 14 review findings across 9 files
flamingo[bot] Sep 14, 2026
edd5371
fix(MESHAGEN-004-2): 14 review findings across 9 files
flamingo[bot] Sep 14, 2026
5a85a2f
fix(MESHAGEN-004-2): 14 review findings across 9 files
flamingo[bot] Sep 14, 2026
6acf59a
fix(MESHAGEN-004-2): 14 review findings across 9 files
flamingo[bot] Sep 14, 2026
30bd297
fix(MESHAGEN-004-2): 14 review findings across 9 files
flamingo[bot] Sep 14, 2026
56e27cf
fix(MESHAGEN-004-2): 14 review findings across 9 files
flamingo[bot] Sep 14, 2026
5bf3327
fix(MESHAGEN-004-2): 14 review findings across 9 files
flamingo[bot] Sep 14, 2026
78eea8f
fix(MESHAGEN-004-2): 14 review findings across 9 files
flamingo[bot] Sep 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions meshcore/MacOS/Install_UI/mac_install_window.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
/*
Copyright 2006 - 2022 Intel Corporation

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

#ifndef MAC_INSTALL_WINDOW_H

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🔴 mac_install_window.h missing required Apache 2.0 license header

Added the standard Apache 2.0 license block-comment header (Intel Corporation copyright, canonical apache.org/licenses/LICENSE-2.0 URL) at the top of meshcore/MacOS/Install_UI/mac_install_window.h, before the #ifndef MAC_INSTALL_WINDOW_H guard, matching the convention used in ILibSimpleDataStore.h, ILibWebRTC.h, and ILibDuktape_NetworkMonitor.h. No other content was changed.

🤖 Prompt for AI agents 
In meshcore/MacOS/Install_UI/mac_install_window.h around line 1, review and complete this code-review fix: mac_install_window.h missing required Apache 2.0 license header.
What the draft fix changed: Added the standard Apache 2.0 license block-comment header (Intel Corporation copyright, canonical apache.org/licenses/LICENSE-2.0 URL) at the top of meshcore/MacOS/Install_UI/mac_install_window.h, before the `#ifndef MAC_INSTALL_WINDOW_H` guard, matching the convention used in ILibSimpleDataStore.h, ILibWebRTC.h, and ILibDuktape_NetworkMonitor.h. No other content was changed.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

#define MAC_INSTALL_WINDOW_H

Expand Down Expand Up @@ -43,3 +59,4 @@ InstallResult show_install_assistant_window(void);
#endif

#endif // MAC_INSTALL_WINDOW_H

16 changes: 16 additions & 0 deletions meshcore/MacOS/TCC_UI/mac_permissions_window.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
/*
Copyright 2006 - 2022 Intel Corporation

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

#ifndef MAC_PERMISSIONS_WINDOW_H

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🔴 mac_permissions_window.h missing required Apache 2.0 license header

Added the standard Apache 2.0 license block comment at the top of meshcore/MacOS/TCC_UI/mac_permissions_window.h, before the #ifndef MAC_PERMISSIONS_WINDOW_H include guard, matching the convention used in other meshcore headers as suggested in the finding. No other content was altered.

🤖 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 missing required Apache 2.0 license header.
What the draft fix changed: Added the standard Apache 2.0 license block comment at the top of meshcore/MacOS/TCC_UI/mac_permissions_window.h, before the `#ifndef MAC_PERMISSIONS_WINDOW_H` include guard, matching the convention used in other meshcore headers as suggested in the finding. No other content was altered.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

#define MAC_PERMISSIONS_WINDOW_H

Expand Down
2 changes: 1 addition & 1 deletion meshcore/MacOS/bundle_detection.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🔴 Missing Apache 2.0 Intel copyright header on meshcore MacOS bundle detection header

Changed the copyright line in the header block at the top of meshcore/MacOS/bundle_detection.h from "Copyright 2025" to "Copyright 2025 Intel Corporation", matching the attribution convention used elsewhere (e.g. modules/win-registry.js, microscript/ILibDuktape_SHA256.c) as required by MESHAGEN-004-2. No other content was altered.

🤖 Prompt for AI agents 
In meshcore/MacOS/bundle_detection.h around line 1, review and complete this code-review fix: Missing Apache 2.0 Intel copyright header on meshcore MacOS bundle detection header.
What the draft fix changed: Changed the copyright line in the header block at the top of meshcore/MacOS/bundle_detection.h from "Copyright 2025" to "Copyright 2025 Intel Corporation", matching the attribution convention used elsewhere (e.g. modules/win-registry.js, microscript/ILibDuktape_SHA256.c) as required by MESHAGEN-004-2. No other content was altered.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 97 high — react 👍/👎 to teach the reviewer

Copyright 2025
Copyright 2025 Intel Corporation

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down
16 changes: 16 additions & 0 deletions meshcore/MacOS/mac_plist_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
/*
Copyright 2024 - 2025 Intel Corporation

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

#ifndef MAC_PLIST_UTILS_H

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🔴 mac_plist_utils.h is missing the required Apache 2.0 license header

Added the Apache 2.0 license header block (Intel Corporation copyright) at the top of meshcore/MacOS/mac_plist_utils.h, before the include guard, matching the format used in other meshcore/microstack headers. No other content was changed.

🤖 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 is missing the required Apache 2.0 license header.
What the draft fix changed: Added the Apache 2.0 license header block (Intel Corporation copyright) at the top of meshcore/MacOS/mac_plist_utils.h, before the include guard, matching the format used in other meshcore/microstack headers. No other content was changed.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

#define MAC_PLIST_UTILS_H

Expand Down
17 changes: 16 additions & 1 deletion modules/_agentStatus.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,18 @@
/*
Copyright 2018-2021 Intel Corporation

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

var promise = require('promise');

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🔴 modules/_agentStatus.js is missing the required Apache 2.0 license header

Added the standard Apache License 2.0 block comment with the Intel Corporation copyright line to the very top of modules/_agentStatus.js, matching the header style used in modules/amt-lme.js, resolving the missing-license-header finding. No other code was changed.

🤖 Prompt for AI agents 
In modules/_agentStatus.js around line 2, review and complete this code-review fix: modules/_agentStatus.js is missing the required Apache 2.0 license header.
What the draft fix changed: Added the standard Apache License 2.0 block comment with the Intel Corporation copyright line to the very top of modules/_agentStatus.js, matching the header style used in modules/amt-lme.js, resolving the missing-license-header finding. No other code was changed.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer

var nodeid = require('_agentNodeId')();
Expand Down Expand Up @@ -95,4 +110,4 @@ function start()
}).then(function (v) { console.log(v); }).then(function () { process._exit(); }).catch(function () { process._exit(); });
}

module.exports = { start: start, query: queryAgent };
module.exports = { start: start, query: queryAgent };
27 changes: 21 additions & 6 deletions modules/macOSHelpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
/*

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🔴 modules/macOSHelpers.js is missing the required Apache 2.0 / Intel copyright header

Replaced the generic "Copyright 2024" comment at the top of modules/macOSHelpers.js with the full Apache License 2.0 block including the "Copyright 2006 - 2024 Intel Corporation" line and the canonical http://www.apache.org/licenses/LICENSE-2.0 URL, matching the suggested fix exactly while preserving the trailing module description lines.

🤖 Prompt for AI agents 
In modules/macOSHelpers.js around line 1, review and complete this code-review fix: modules/macOSHelpers.js is missing the required Apache 2.0 / Intel copyright header.
What the draft fix changed: Replaced the generic "Copyright 2024" comment at the top of modules/macOSHelpers.js with the full Apache License 2.0 block including the "Copyright 2006 - 2024 Intel Corporation" line and the canonical http://www.apache.org/licenses/LICENSE-2.0 URL, matching the suggested fix exactly while preserving the trailing module description lines.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer

Copyright 2024
Copyright 2006 - 2024 Intel Corporation

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.

macOS Platform Helper Functions
Centralizes macOS-specific utilities for bundle detection, service naming, and system operations
Expand Down Expand Up @@ -169,19 +181,22 @@ function copyBundleWithDitto(sourcePath, targetPath) {
var child_process = require('child_process');
var fs = require('fs');

var dittoError = null;

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🟠 copyBundleWithDitto reads stderr synchronously but does not accumulate output via the standard .str pattern used elsewhere in the codebase

In copyBundleWithDitto, changed stderr accumulation to follow the .str idiom: child.stderr.str = '' is initialized before the listener, and the 'data' handler now does this.str += d.toString() instead of overwriting a closured dittoError variable, so multiple stderr chunks are fully accumulated. dittoError is now derived from child.stderr.str after waitExit(). Risk: relies on this inside the event handler correctly referring to child.stderr (i.e., the emitter), which is standard EventEmitter behavior but assumes no .bind()/arrow-function usage elsewhere overrides it.

🤖 Prompt for AI agents 
In modules/macOSHelpers.js around line 172, review and complete this code-review fix: copyBundleWithDitto reads stderr synchronously but does not accumulate output via the standard .str pattern used elsewhere in the codebase.
What the draft fix changed: In copyBundleWithDitto, changed stderr accumulation to follow the `.str` idiom: `child.stderr.str = ''` is initialized before the listener, and the 'data' handler now does `this.str += d.toString()` instead of overwriting a closured `dittoError` variable, so multiple stderr chunks are fully accumulated. `dittoError` is now derived from `child.stderr.str` after `waitExit()`. Risk: relies on `this` inside the event handler correctly referring to `child.stderr` (i.e., the emitter), which is standard EventEmitter behavior but assumes no `.bind()`/arrow-function usage elsewhere overrides it.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 60 medium — react 👍/👎 to teach the reviewer

var child = child_process.execFile(MACOS_PATHS.DITTO, ['ditto', sourcePath, targetPath]);

child.stderr.str = '';
child.stderr.on('data', function(d) {
dittoError = d.toString();
this.str += d.toString();
process.stderr.write(d);
});

child.waitExit();

// Verify the copy succeeded by checking if target exists
if (dittoError || !fs.existsSync(targetPath)) {
throw new Error('Bundle copy failed: ' + (dittoError || 'Target not created'));
var dittoError = child.stderr.str || null;
var exitCode = child.exitCode;

// Verify the copy succeeded by checking the exit code, stderr output, and target existence
if ((exitCode !== undefined && exitCode !== null && exitCode !== 0) || dittoError || !fs.existsSync(targetPath)) {
throw new Error('Bundle copy failed: ' + (dittoError || ('Target not created (exit code: ' + exitCode + ')')));
}

return true;
Comment on lines 181 to 202

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🟠 copyBundleWithDitto swallows non-zero exit codes from ditto and only checks stderr output / target existence

In copyBundleWithDitto, added a check of the child's actual exit code (child.exitCode) alongside the existing stderr/target-existence checks, so a non-zero exit code now triggers failure even when stderr is empty. Confidence is limited because the exact property name/API used by this codebase's child_process wrapper for exit status (child.exitCode) is inferred from convention rather than confirmed against this repo's execFile implementation; if the actual property differs (e.g. child.status or an argument to waitExit), this check will be a no-op.

🤖 Prompt for AI agents 
In modules/macOSHelpers.js around line 168, review and complete this code-review fix: copyBundleWithDitto swallows non-zero exit codes from ditto and only checks stderr output / target existence.
What the draft fix changed: In copyBundleWithDitto, added a check of the child's actual exit code (child.exitCode) alongside the existing stderr/target-existence checks, so a non-zero exit code now triggers failure even when stderr is empty. Confidence is limited because the exact property name/API used by this codebase's child_process wrapper for exit status (child.exitCode) is inferred from convention rather than confirmed against this repo's execFile implementation; if the actual property differs (e.g. child.status or an argument to waitExit), this check will be a no-op.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer

Expand Down
Loading