feat(obd_ai): expose diagnostic tools via callable server - #14
virtuscyber wants to merge 1 commit into
Conversation
🤖 Augment PR SummarySummary: This PR adds a runnable “tool server” layer on top of Changes:
Technical Notes: The server intentionally limits the exposed surface to safe, read-only diagnostic operations and reuses the same structured payloads/errors as the in-process tool API. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
|
|
||
| if self.path == "/call": | ||
| tool_name = payload.get("tool") | ||
| input_payload = payload.get("input", {}) |
There was a problem hiding this comment.
POST /call accepts any JSON type for the input field, but OBDAIReadOnlyToolSurface methods do dict(input or {}), so non-object inputs (e.g., [1,2]) will raise and get reported as a generic server_error rather than a 400-level client error. Consider validating input is a JSON object here (similar to _read_json_body) so malformed requests reliably return invalid_request.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| """Create a threaded HTTP server exposing the read-only tool surface.""" | ||
|
|
||
| app = OBDAIFunctionServer(surface=surface) | ||
| server = ThreadingHTTPServer((host, port), _OBDAIHTTPRequestHandler) |
There was a problem hiding this comment.
ThreadingHTTPServer will process requests concurrently against a single shared OBDAIFunctionServer/OBDAIReadOnlyToolSurface instance, but the surface holds mutable session state (self._session) that doesn’t appear thread-safe. Concurrent calls (e.g., connect_vehicle racing with read_sensor) could lead to inconsistent session/connection behavior or closed-while-in-use errors.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| return 0 | ||
|
|
||
| if args.command == "call": | ||
| payload = json.loads(args.input_json) |
There was a problem hiding this comment.
In the CLI call path, json.loads(args.input_json) will raise JSONDecodeError on invalid JSON and print a stack trace, whereas the HTTP server returns a structured invalid_json payload. Consider handling decode failures here so CLI users get a clear, consistent error message.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| base_url = f"http://127.0.0.1:{server.server_port}" | ||
| try: | ||
| with urllib.request.urlopen(f"{base_url}/tools") as response: |
There was a problem hiding this comment.
These urllib.request.urlopen(...) calls don’t set a timeout, so a server regression (or port bind issue) can cause the test to hang until the overall CI timeout rather than failing quickly. Consider adding a small timeout to both requests to make the test suite more robust.
Severity: medium
Other Locations
tests/test_obd_ai_server.py:113
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
1 participant
Refs #5
Stacked on top of #4.
Includes: