-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix: Executing cli-anything-browser --json fs ls yields no return res… #218
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -26,6 +26,8 @@ | |||||||||||||||||||||||||||||||||||||||||
| # DOMSHELL_TOKEN — auth token (required, must match the running server) | ||||||||||||||||||||||||||||||||||||||||||
| # DOMSHELL_PORT — MCP HTTP port of the running server (default: 3001) | ||||||||||||||||||||||||||||||||||||||||||
| DEFAULT_SERVER_CMD = "npx" | ||||||||||||||||||||||||||||||||||||||||||
| MCP_CALL_TIMEOUT_SECONDS = float(os.environ.get("DOMSHELL_MCP_CALL_TIMEOUT", "15")) | ||||||||||||||||||||||||||||||||||||||||||
| MCP_CLOSE_TIMEOUT_SECONDS = float(os.environ.get("DOMSHELL_MCP_CLOSE_TIMEOUT", "2")) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+29
to
32
|
||||||||||||||||||||||||||||||||||||||||||
| MCP_CALL_TIMEOUT_SECONDS = float(os.environ.get("DOMSHELL_MCP_CALL_TIMEOUT", "15")) | |
| MCP_CLOSE_TIMEOUT_SECONDS = float(os.environ.get("DOMSHELL_MCP_CLOSE_TIMEOUT", "2")) | |
| def _get_timeout_seconds(env_var_name: str, default: float) -> float: | |
| """Parse a positive timeout value from the environment, with fallback.""" | |
| raw_value = os.environ.get(env_var_name) | |
| if raw_value is None: | |
| return default | |
| try: | |
| timeout = float(raw_value) | |
| except (TypeError, ValueError): | |
| return default | |
| if timeout <= 0: | |
| return default | |
| return timeout | |
| MCP_CALL_TIMEOUT_SECONDS = _get_timeout_seconds("DOMSHELL_MCP_CALL_TIMEOUT", 15.0) | |
| MCP_CLOSE_TIMEOUT_SECONDS = _get_timeout_seconds("DOMSHELL_MCP_CLOSE_TIMEOUT", 2.0) |
Copilot
AI
Apr 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new docstrings/comments here are in Chinese while the rest of this module’s docstrings are in English. To keep the harness maintainable for the broader contributor base, please translate these docstrings/comments to English (or keep bilingual text consistently across the file).
Copilot
AI
Apr 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_run_sync() calls get_event_loop_policy().get_event_loop() to capture the previous loop. In Python 3.11+, this can implicitly create a new event loop when none exists, and then you restore it at the end without closing it, leaking a loop for the rest of the process. Consider restoring to None unless there was an explicitly set loop, or using get_running_loop() to detect an active loop and otherwise leaving the policy’s current loop unset.
| old_loop = asyncio.get_event_loop_policy().get_event_loop() | |
| old_loop = asyncio.get_running_loop() |
Copilot
AI
Apr 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_call_tool() is now returning normalized dicts (via _normalize_tool_result) rather than the raw MCP CallToolResult, but its signature/docstring still imply it returns the MCP server result unchanged (-> Any). Please update the type hint/docstring to reflect the new return contract so callers and tests don’t have to guess.
Copilot
AI
Apr 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one-shot code path manually calls aenter on stdio_client / ClientSession but never calls aexit for either context. That can leak subprocesses/pipes (and potentially keep an orphaned npx/domshell-proxy running after the CLI prints output). Consider adding a finally block that closes both contexts using the new _safe_aexit() (with timeout) so you still avoid the observed hang while ensuring resources are released.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These new test docstrings are in Chinese while the surrounding test file uses English docstrings/comments. For consistency and easier maintenance by the rest of the team, please translate these docstrings to English (or adopt a consistent bilingual style across the file).