Skip to content

feature: compile nmap#130

Closed
m0hamed-ait wants to merge 2 commits into
mainfrom
feature/compile_nmap
Closed

feature: compile nmap#130
m0hamed-ait wants to merge 2 commits into
mainfrom
feature/compile_nmap

Conversation

@m0hamed-ait

@m0hamed-ait m0hamed-ait commented May 20, 2026

Copy link
Copy Markdown

Add support for compiling the Nmap agent using Nuitka to produce standalone binaries.

Usage :

  • Build the compiled variant:
    ostorlab agent build --file=ostorlab.compiled.yaml -o <org>
  • Build the standard (source) variant (unchanged):
    ostorlab agent build --file=ostorlab.yaml -o <org>

@m0hamed-ait
m0hamed-ait requested a review from a team as a code owner May 20, 2026 15:53
@codecov

codecov Bot commented May 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 94.04%. Comparing base (58e79d5) to head (66aa09f).

Files with missing lines Patch % Lines
agent/nmap_agent.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #130      +/-   ##
==========================================
+ Coverage   94.03%   94.04%   +0.01%     
==========================================
  Files          11       11              
  Lines        1040     1042       +2     
==========================================
+ Hits          978      980       +2     
  Misses         62       62              

☔ 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.

@ostorlab-ai-pr-review ostorlab-ai-pr-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Total Issues Found: 1
Critical Issues: 0
Suggestions: 1

Key Findings:
One suggestion was raised regarding command construction in agent/mcp/runner.py (line 10). The reviewer recommends conditionally building the execution command to explicitly invoke python3.14 for non-compiled paths rather than relying on shebang lines or execute permissions. This is a reliability and robustness improvement to ensure the correct interpreter is always used, avoiding potential runtime failures due to environment-specific permission or shebang issues.

Key Themes and Patterns:

  • Explicit interpreter invocation for interpreted code paths
  • Reliability of command execution in mixed (compiled vs. non-compiled) environments
  • Reducing dependency on ambient file permissions and shebang behavior

Overall Code Quality Assessment:
The code appears functional, but this comment highlights a minor maintainability gap. Addressing it will make the runner more deterministic across different deployment environments.

General Recommendations:

  • Accept the suggestion to explicitly specify the interpreter when constructing execution commands.
  • Ensure the conditional logic cleanly handles both compiled and interpreted entry points.
  • Add a brief inline comment explaining why explicit interpreter invocation is used.

Comment thread agent/mcp/runner.py


def run() -> None:
def run(is_compiled: bool = False) -> None:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Conditionally construct the command to preserve the interpreter for the interpreted path, explicitly invoking python3.14 for the non-compiled case instead of relying on shebang/execute permissions.

Suggested change
def run(is_compiled: bool = False) -> None:
if is_compiled:
cmd = [RUN_SERVER_PATH_COMPILED]
else:
cmd = ["python3.14", RUN_SERVER_PATH]

@m0hamed-ait
m0hamed-ait deleted the feature/compile_nmap branch May 20, 2026 16:41
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.

1 participant