MCP Bridge Fixes & Dev Environment Automation #1480
Conversation
…entic-workflows # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
📝 WalkthroughWalkthroughThis PR removes Changes
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
server/api_server/openapi/schemas.py (1)
720-745:⚠️ Potential issue | 🟠 MajorRemove
pattern=MAC_PATTERNfrom themacfield—the field validatorvalidate_mac()already handles regex validation.The
patternconstraint rejects"Internet"before the field validator runs (Pydantic validates JSON schema constraints before field validators). Sincevalidate_mac()explicitly allows"Internet"as a special case for the WAN/gateway device, the regex pattern should not appear here.- mac: str = Field(..., description="Device MAC", pattern=MAC_PATTERN) + mac: str = Field(..., description="Device MAC")docs/API_MCP.md (1)
45-52:⚠️ Potential issue | 🟡 MinorUpdate the OpenAPI spec endpoint path in the sequence diagram to match the canonical route.
The diagram showsGET /mcp/sse/openapi.json, but the actual Flask route registered is/openapi.json(api_server_start.py:1171). Update the diagram to use the canonical path for consistency.
🤖 Fix all issues with AI agents
In @.gemini/skills/testing-workflow/SKILL.md:
- Around line 54-57: In the "Authentication & Environment Reset" section fix the
missing space by changing the contiguous token "APIs.After" to "APIs. After" so
the sentence reads correctly; update the paragraph that mentions `API_TOKEN`
under the "Authentication & Environment Reset" heading to include the space
after the period.
- Around line 11-23: Add a new pre-run step that invokes testFailure() before
running the test suite: insert a "0.5 Pre-run: Capture current failures" section
immediately after the "## 0. Pre-requisites: Environment Check" block in
.gemini/skills/testing-workflow/SKILL.md and include the single-line invocation
testFailure() so the workflow captures current failures before executing tests.
🧹 Nitpick comments (2)
.devcontainer/scripts/generate-configs.sh (1)
34-45: Harden permissions for generated token configs.
These files store bearer tokens; set restrictive permissions after writing to reduce accidental exposure in shared volumes.🔒 Suggested hardening
jq --arg t "$TOKEN" '.mcpServers["netalertx-devcontainer"] = {url: "http://127.0.0.1:20212/mcp/sse", headers: {Authorization: ("Bearer " + $t)}}' "${ROOT_DIR}/.gemini/settings.json" > "${ROOT_DIR}/.gemini/settings.json.tmp" && mv "${ROOT_DIR}/.gemini/settings.json.tmp" "${ROOT_DIR}/.gemini/settings.json" + chmod 600 "${ROOT_DIR}/.gemini/settings.json" # VS Code MCP config mkdir -p "${ROOT_DIR}/.vscode" [ -f "${ROOT_DIR}/.vscode/mcp.json" ] || echo "{}" > "${ROOT_DIR}/.vscode/mcp.json" jq --arg t "$TOKEN" '.servers["netalertx-devcontainer"] = {type: "sse", url: "http://127.0.0.1:20212/mcp/sse", headers: {Authorization: ("Bearer " + $t)}}' "${ROOT_DIR}/.vscode/mcp.json" > "${ROOT_DIR}/.vscode/mcp.json.tmp" && mv "${ROOT_DIR}/.vscode/mcp.json.tmp" "${ROOT_DIR}/.vscode/mcp.json" + chmod 600 "${ROOT_DIR}/.vscode/mcp.json"server/api_server/api_server_start.py (1)
769-791: Consider adding a constant or validation for the totals list structure.The index-based extraction with defensive bounds checking is reasonable, but the mapping relies on an implicit contract with
getTotals()that returns[devices, connected, favorites, new, down, archived]. If the order changes or items are added/removed, this will silently return incorrect data or zeros.The static analysis warning about unused
payload(line 778) can be safely ignored—it follows the pattern used by other endpoints where the@validate_requestdecorator may inject validated payload.💡 Optional: Document or validate the structure
def api_devices_totals_named(payload=None): device_handler = DeviceInstance() totals_list = device_handler.getTotals() - # totals_list order: [devices, connected, favorites, new, down, archived] + # Expected totals_list order: [devices, connected, favorites, new, down, archived] + EXPECTED_TOTALS_LENGTH = 6 + if len(totals_list) != EXPECTED_TOTALS_LENGTH: + mylog("verbose", [f"[api_devices_totals_named] Unexpected totals length: {len(totals_list)}, expected {EXPECTED_TOTALS_LENGTH}"]) totals_dict = {
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.gemini/skills/testing-workflow/SKILL.md (1)
78-78:⚠️ Potential issue | 🟡 MinorAdd working directory to troubleshooting command.
Same issue as the token retrieval step above: the python command needs to be run from the project root directory.
📂 Proposed fix to add working directory
-3. Verify Python can read it: `python3 -c "from helper import get_setting_value; print(get_setting_value('API_TOKEN'))"` +3. Verify Python can read it: `cd /workspaces/NetAlertX; python3 -c "from helper import get_setting_value; print(get_setting_value('API_TOKEN'))"`
🤖 Fix all issues with AI agents
In @.gemini/skills/testing-workflow/SKILL.md:
- Around line 66-69: The token-retrieval example uses a relative import (from
helper import get_setting_value) which can fail if not run from project root;
update the command in SKILL.md to change to the project working directory first
(e.g., cd /workspaces/NetAlertX) before running the python3 -c invocation so
helper.get_setting_value is importable; ensure the modified line mirrors other
examples that explicitly switch to /workspaces/NetAlertX.
|
@jokob-sk you can run the generate container configs task now to give AI agents such as Copilot or Gemini access to the NetAlertX API. it will generate a project settings for Gemini and a mcp.json for Copilot. This will allow testing, manipulating, and helping with UI work. |
2 participants
Summary
This PR stabilizes the MCP (Model Context Protocol) bridge, automates development environment configuration for AI agents, and cleans up API documentation and schemas.
Key Changes
🛠️ Fixes
mcp_endpoint.py):/metrics) caused JSON parsing errors.ValueErroralongsideJSONDecodeError.schemas.py):CreateSessionRequestandCreateEventRequestwith strict MAC address regex validation (pattern=MAC_PATTERN).create_device_eventpath parameters.⚡ Enhancements
generate-configs.sh):.gemini/settings.jsonand.vscode/mcp.jsonif anAPI_TOKENis detected..gemini/settings.jsonand.vscode/mcp.jsonto prevent secret leakage.📚 Documentation
API_MCP.md):/mcp/sse/prefixes).{mac}matching OpenAPI standards.run_nmap_scanpopulatesget_open_ports).linkstorun_nmap_scanandget_open_portsdefinitions to help agents discover related tools.Testing
get_metricsnow returns plain text successfully.pytest test/api_endpoints/test_mcp_tools_endpoints.pypassed).Summary by CodeRabbit
New Features
Documentation
Improvements