Standardize and Optimize OpenAPI & MCP for AI Agents #1476
Merged
+746
−143
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
📝 WalkthroughWalkthroughAdds a global 500 error handler, regenerates and applies MCP_DISABLED_TOOLS at startup via get_openapi_spec, deduplicates MCP tools by original operationId with prioritization, enriches OpenAPI metadata (response content types, links, query/path params), enhances schemas/validation, and updates tests accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant FlaskApp as Flask App
participant SpecGen as OpenAPI Spec Generator
participant Env as Env/Settings
participant Registry as Tool Registry
participant MCP as MCP Mapper
rect rgba(135,206,250,0.5)
FlaskApp->>SpecGen: get_openapi_spec(force_refresh=True, flask_app)
SpecGen->>Env: read MCP_DISABLED_TOOLS (settings or env or default)
SpecGen->>Registry: build OpenAPI paths & operations
SpecGen->>MCP: map_openapi_to_mcp_tools(spec)
MCP->>MCP: deduplicate by x-original-operationId (prefer /mcp/ and POST)
MCP->>SpecGen: return mapped tools (with x-mcp-disabled flags)
SpecGen->>FlaskApp: return final OpenAPI spec
end
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@server/api_server/api_server_start.py`:
- Around line 104-113: The global exception handler handle_500_error is catching
HTTPException instances (e.g., abort(400/403/405)) and turning them into 500s;
modify handle_500_error to early-return the original exception when
isinstance(e, HTTPException) so Flask preserves its status/code, and add the
required import from werkzeug.exceptions import HTTPException; keep the existing
logging and 500 JSON response for non-HTTP exceptions.
In `@server/api_server/openapi/schemas.py`:
- Around line 679-693: Add the missing Pydantic model declaration by wrapping
the orphaned docstring, model_config and fields in a class named SessionInfo
that inherits from BaseModel; specifically create class SessionInfo(BaseModel):
above the existing docstring so the docstring "Session information.", the
model_config ConfigDict (with extra="allow" and json_schema_extra examples) and
any ses*-prefixed field definitions become members of SessionInfo, ensuring
imports for BaseModel/ConfigDict are present if not already.
In `@server/api_server/openapi/spec_generator.py`:
- Around line 77-106: The code never reads the MCP_DISABLED_TOOLS environment
variable because disabled_env is initialized to "" and later checked with "is
None"; change the control flow in the disabled-tools block so the env var is
consulted when the helper setting is absent or empty: e.g., initialize
disabled_env as None or treat empty helper setting as missing, call
os.getenv("MCP_DISABLED_TOOLS") when setting_val is falsy (or disabled_env is
None/empty) before falling back to the hard-coded default; keep calling
set_tool_disabled(op, True) for each parsed op and preserve the outer try/except
behavior. Reference symbols: disabled_env, get_setting_value,
MCP_DISABLED_TOOLS, os.getenv, and set_tool_disabled.
🧹 Nitpick comments (3)
server/api_server/mcp_endpoint.py (1)
413-424: Remove unused_tool_name_to_operation_idvariable.The
_tool_name_to_operation_iddictionary is populated on line 418 but never used or returned. This appears to be leftover from development or an incomplete feature.🧹 Proposed cleanup
final_tools = [] - _tool_name_to_operation_id: Dict[str, str] = {} for tool in tools_map.values(): - actual_operation_id = tool["name"] # Save before overwriting tool["name"] = tool["_original_op_id"] - _tool_name_to_operation_id[tool["name"]] = actual_operation_id del tool["_original_op_id"] del tool["_is_mcp"] del tool["_is_post"] final_tools.append(tool)server/api_server/openapi/schema_converter.py (1)
200-206: Remove or use the unusedmethodparameter.The
methodparameter is declared but never used in the function body, as flagged by static analysis (ARG001). Either remove it or add a comment indicating it's reserved for future use.🧹 Option 1: Remove unused parameter
def build_responses( response_model: Optional[Type[BaseModel]], definitions: Dict[str, Any], response_content_types: Optional[List[str]] = None, - links: Optional[Dict[str, Any]] = None, - method: str = "post" + links: Optional[Dict[str, Any]] = None ) -> Dict[str, Any]:🧹 Option 2: Document as reserved
def build_responses( response_model: Optional[Type[BaseModel]], definitions: Dict[str, Any], response_content_types: Optional[List[str]] = None, links: Optional[Dict[str, Any]] = None, - method: str = "post" + method: str = "post" # Reserved for future method-specific response handling ) -> Dict[str, Any]: + _ = method # Silence unused parameter warningserver/api_server/openapi/schemas.py (1)
472-509: Consider extracting the validation error message to a constant.The model validator at line 508 uses a detailed inline message. While the static analysis hint (TRY003) is pedantic, extracting this to a constant would improve maintainability if the message needs to change.
💡 Optional: Extract error message
+_WOL_MAC_OR_IP_REQUIRED = "Either devMac (aka mac) or devLastIP (aka ip) must be provided" + class WakeOnLanRequest(BaseModel): # ... fields ... `@model_validator`(mode="after") def require_mac_or_ip(self) -> "WakeOnLanRequest": """Ensure at least one of mac or devLastIP is provided.""" if self.mac is None and self.devLastIP is None: - raise ValueError("Either devMac (aka mac) or devLastIP (aka ip) must be provided") + raise ValueError(_WOL_MAC_OR_IP_REQUIRED) return self
jokob-sk
reviewed
Jan 31, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@server/api_server/openapi/schemas.py`:
- Around line 60-63: ALLOWED_EVENT_TYPES currently includes "Device Up", which
is not a valid NetAlertX event; update the Literal definition named
ALLOWED_EVENT_TYPES to remove "Device Up" and ensure it exactly matches the
canonical NetAlertX event list ("Device Down", "New Device", "Connected",
"Disconnected", "IP Changed", "Down Reconnected" plus the actual missing event
name); locate ALLOWED_EVENT_TYPES in this module and either remove the "Device
Up" entry or replace the list with the authoritative set from NetAlertX (fill in
the correct missing event string from the source of truth).
🧹 Nitpick comments (1)
test/api_endpoints/test_mcp_disabled_tools.py (1)
33-40: Consider renaming unused loop variables to indicate intent.The loop variables
pathandmethodare not used within the loop body. Rename them to_pathand_methodto signal they are intentionally unused.♻️ Proposed fix
found = False - for path, methods in spec["paths"].items(): - for method, op in methods.items(): + for _path, methods in spec["paths"].items(): + for _method, op in methods.items(): if op["operationId"] == "search_devices_api": assert op.get("x-mcp-disabled") is True found = TrueAnd similarly for the second test:
found_read = False - for path, methods in spec["paths"].items(): - for method, op in methods.items(): + for _path, methods in spec["paths"].items(): + for _method, op in methods.items(): if op["operationId"] == "dbquery_read": assert op.get("x-mcp-disabled") is True found_read = TrueAlso applies to: 55-62
jokob-sk
reviewed
Jan 31, 2026
| description="Present in last scan (0 or 1)", | ||
| json_schema_extra={"enum": [0, 1]} | ||
| ) | ||
| devStatus: Optional[Literal["online", "offline"]] = Field( |
Collaborator
There was a problem hiding this comment.
This is a read-only field (= dynamically evaluated - there is no corresponding DB coilumn) - not sure if this causes issues
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This update focuses on making the NetAlertX API more predictable, efficient, and navigable for AI agents and developers alike. We have standardized how the API communicates errors, improved the logical grouping of tools, and added context that allows AI agents to discover workflows autonomously.
Key Improvements
🚀 Enhanced AI Workflows and Tool Selection
_1,_2) with descriptive names (like_mcpor_post). This ensures that both the documentation and the agent's toolbelt are logical and easy to read.📖 Refined Documentation and Schemas
macacross all tools. This consistency removes the friction of agents having to choose betweendevMac,mac, or other variations.HTTP 422 Unprocessable Entityfor semantic errors across all request types.⚡ Optimization and Efficiency
openapi.jsonis now significantly smaller. This reduces token overhead for AI agents and improves documentation performance.Summary by CodeRabbit
Bug Fixes
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.