fix(browser): add MCP timeout guard to prevent fs hangs by marcelsafin · Pull Request #233 · HKUDS/CLI-Anything

marcelsafin · 2026-04-19T18:46:58Z

Description

Add timeout guards around browser MCP initialize/tool calls to prevent indefinite hangs for commands like cli-anything-browser --json fs ls when backend calls stall.

Fixes #217

Type of Change

New Software CLI (in-repo) — adds a CLI harness inside this monorepo
New Software CLI (standalone repo) — registry-only PR pointing to an external repo
New Feature — adds new functionality to an existing harness or the plugin
Bug Fix — fixes incorrect behavior
Documentation — updates docs only
Other — please describe:

For New Software CLIs (in-repo)

<SOFTWARE>.md SOP document exists at <software>/agent-harness/<SOFTWARE>.md
Canonical SKILL.md exists at skills/cli-anything-<software>/SKILL.md
Packaged compatibility SKILL.md exists at cli_anything/<software>/skills/SKILL.md
Unit tests at cli_anything/<software>/tests/test_core.py are present and pass without backend
E2E tests at cli_anything/<software>/tests/test_full_e2e.py are present
README.md includes the new software (with link to harness directory)
registry.json includes an entry with source_url: null (see Contributing guide)
repl_skin.py in utils/ is an unmodified copy from the plugin

For New Software CLIs (standalone repo)

CLI is installable via pip install <package-name> or a pip install git+https://... URL
SKILL.md exists in the external repo
External repo has its own test suite
registry.json entry includes source_url pointing to the external repo
registry.json entry includes skill_md with full URL to the external SKILL.md
install_cmd in registry.json works (tested locally)

For Existing CLI Modifications

All unit tests pass: python3 -m pytest cli_anything/<software>/tests/test_core.py -v
All E2E tests pass: python3 -m pytest cli_anything/<software>/tests/test_full_e2e.py -v (suite executes; skipped without external DOMShell env)
No test regressions — no previously passing tests were removed or weakened
registry.json entry is updated if version, description, or requirements changed

General Checklist

Code follows existing patterns and conventions
--json flag is supported on any new commands
Commit messages follow the conventional format (feat:, fix:, docs:, test:)
I have tested my changes locally

Test Results

$ /tmp/CLI-Anything/.venv/bin/python -m pytest browser/agent-harness/cli_anything/browser/tests/test_core.py -v
36 passed

$ /tmp/CLI-Anything/.venv/bin/python -m pytest browser/agent-harness/cli_anything/browser/tests/test_security.py -v
49 passed

$ /tmp/CLI-Anything/.venv/bin/python -m pytest browser/agent-harness/cli_anything/browser/tests/test_full_e2e.py -v
10 skipped

marcelsafin · 2026-04-19T19:06:50Z

@maintainers Could you switch requested reviewer to Codex for this PR? Copilot review is currently unavailable on our side.

marcelsafin

Codex review pass completed.

Result: No blocking issues found in this PR.

Checked:

Timeout guard is applied to init + tool calls in daemon and non-daemon paths
Error messaging is actionable for token/server reachability problems
New tests cover default/invalid/min timeout parsing and timeout exception path
Registry version bump aligns with behavior and docs changes

Residual risk: low (mainly environment-dependent DOMShell runtime behavior).

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

marcelsafin · 2026-04-19T19:29:37Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a5c940da61

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

marcelsafin · 2026-04-19T19:36:39Z

@codex address that feedback. Please ensure daemon timeout RuntimeError is propagated and does not trigger automatic non-daemon retry/reissue.

chatgpt-codex-connector · 2026-04-19T19:36:45Z

To use Codex here, create an environment for this repo.

marcelsafin · 2026-04-19T19:40:01Z

Addressed the previous Codex feedback in the latest commit. @codex review

chatgpt-codex-connector · 2026-04-19T19:40:05Z

To use Codex here, create an environment for this repo.

marcelsafin · 2026-04-19T19:59:07Z

@codex review

marcelsafin · 2026-04-19T20:01:44Z

@codex review latest head (3bcd80c) only. Previous P2 timeout-retry feedback has been addressed and thread resolved.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3bcd80c7e9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

marcelsafin · 2026-04-19T20:08:14Z

@codex review

marcelsafin · 2026-04-19T20:10:44Z

@codex review latest head cd55247 for timeout/fallback regression only.

marcelsafin · 2026-04-19T20:11:57Z

@maintainers Status update: latest Codex P1 feedback is addressed in cd55247 and thread is resolved. Verification rerun:

pytest browser/agent-harness/cli_anything/browser/tests/test_core.py -q → 38 passed
pytest browser/agent-harness/cli_anything/browser/tests/test_security.py -q → 49 passed
Could you please take final maintainer review/approval and run required gates?

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cd55247595

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

marcelsafin · 2026-04-19T20:14:54Z

@codex review latest head 5338d01 (timeout recovery behavior).

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5338d01d82

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-19T20:17:35Z

-        except Exception as e:
+        except MCPToolTimeoutError:
+            # Do not auto-retry this tool call, but reset daemon for future recovery.
+            await _stop_daemon()


Bound daemon reset in timeout handler

Calling await _stop_daemon() directly in the MCPToolTimeoutError path can reintroduce an indefinite hang when the timeout was caused by a wedged daemon transport: _stop_daemon() awaits __aexit__ calls without any timeout, so cleanup itself may block forever and the tool never returns the timeout error. This means the new timeout-recovery path can still stall user commands under the same unresponsive-daemon conditions it is meant to recover from.

Useful? React with 👍 / 👎.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-20T21:11:20Z

+                    tool_name,
+                )
                return result
+    except RuntimeError:


In _call_tool, except RuntimeError: raise is overly broad and will allow unrelated RuntimeErrors from session.initialize() / session.call_tool() to bypass the helpful wrapping RuntimeError("DOMShell MCP call failed...") message below. Consider narrowing this to only pass through MCPToolTimeoutError (or explicitly re-raising that type) while still wrapping other exceptions to preserve consistent, actionable error context for non-timeout failures.

Suggested change

except RuntimeError:

except MCPToolTimeoutError:

Copilot · 2026-04-20T21:11:20Z

+            await _await_with_timeout(
+                _daemon_session.call_tool("domshell_focus", {"name": path}),
+                "domshell_focus (daemon)",
+            )
+            return await _await_with_timeout(
+                _daemon_session.call_tool("domshell_type", {"text": text}),
+                "domshell_type (daemon)",
+            )


type_text() uses the daemon session directly; if a daemon call times out (MCPToolTimeoutError), the daemon is not reset, unlike _call_tool which calls _stop_daemon() to allow recovery on subsequent commands. Consider catching MCPToolTimeoutError in the daemon branch here, calling _stop_daemon(), then re-raising so the next command can reconnect cleanly.

Suggested change

await _await_with_timeout(

_daemon_session.call_tool("domshell_focus", {"name": path}),

"domshell_focus (daemon)",

)

return await _await_with_timeout(

_daemon_session.call_tool("domshell_type", {"text": text}),

"domshell_type (daemon)",

)

try:

await _await_with_timeout(

_daemon_session.call_tool("domshell_focus", {"name": path}),

"domshell_focus (daemon)",

)

return await _await_with_timeout(

_daemon_session.call_tool("domshell_type", {"text": text}),

"domshell_type (daemon)",

)

except MCPToolTimeoutError:

await _stop_daemon()

raise

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-20T21:40:51Z

      "name": "browser",
      "display_name": "Browser",
-      "version": "1.0.0",
+      "version": "1.0.1",


registry.json bumps browser to 1.0.1, but the browser harness/package still reports 1.0.0 (e.g., browser/agent-harness/setup.py, cli_anything/browser/__init__.py, and the REPL banner in browser_cli.py). Please keep these versions in sync so users don’t see conflicting versions between cli-hub/registry and the installed package/CLI output.

Suggested change

"version": "1.0.1",

"version": "1.0.0",

marcelsafin · 2026-04-20T22:33:22Z

@copilot apply changes based on the comments in this thread

marcelsafin · 2026-04-21T07:10:36Z

@copilot apply changes based on the comments in this thread

…y, sync version Addresses Copilot/codex review feedback: - Bound _stop_daemon() __aexit__ calls with asyncio.wait_for so timeout recovery cannot itself hang on a wedged daemon transport (DOMSHELL_DAEMON_STOP_TIMEOUT, default 5s). - Narrow 'except RuntimeError: raise' to only pass through MCPToolTimeoutError; other RuntimeErrors are still wrapped with the helpful 'Ensure Chrome is running...' context. - Add MCPToolTimeoutError handling to type_text() daemon branch so a timeout there also resets the daemon, matching _call_tool behavior. - Bump browser package version 1.0.0 -> 1.0.1 to match registry.json (setup.py, browser/__init__.py, browser_cli.py REPL banner). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

marcelsafin · 2026-04-23T14:28:40Z

Addressed Copilot + codex review feedback in 7426cbb:

Bound _stop_daemon() with asyncio.wait_for (default 5s, env DOMSHELL_DAEMON_STOP_TIMEOUT) so cleanup itself cannot hang on a wedged transport.
Narrowed except RuntimeError to only pass through MCPToolTimeoutError; other RuntimeErrors are wrapped with the helpful 'Ensure Chrome is running…' context.
Added MCPToolTimeoutError handling to type_text() daemon branch so a timeout there also resets the daemon, matching _call_tool behavior.
Bumped browser package 1.0.0 → 1.0.1 in setup.py, __init__.py, and the REPL banner to match registry.json.

@copilot review

yuh-yang · 2026-04-29T05:18:04Z

Thanks for the timeout guard. The timeout parsing and cleanup shape look good, but I think the default 20 second wrapper around session.initialize() may be too short for the first npx @apireno/domshell run, especially when it has to download the package.

Please use a longer/separate initialization timeout or only start the shorter timeout once the package is already available.

fix(browser): add MCP timeout guard to prevent fs hangs

a5c940d

Copilot AI review requested due to automatic review settings April 19, 2026 18:46

Copilot started reviewing on behalf of marcelsafin April 19, 2026 18:47 View session

marcelsafin commented Apr 19, 2026

View reviewed changes

Copilot AI reviewed Apr 19, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Apr 19, 2026

View reviewed changes

Comment thread browser/agent-harness/cli_anything/browser/utils/domshell_backend.py

fix(browser): avoid retrying daemon timeouts

3bcd80c

chatgpt-codex-connector Bot reviewed Apr 19, 2026

View reviewed changes

Comment thread browser/agent-harness/cli_anything/browser/utils/domshell_backend.py Outdated

fix(browser): restrict daemon no-retry to timeout errors

cd55247

Copilot AI review requested due to automatic review settings April 19, 2026 20:08

Copilot started reviewing on behalf of marcelsafin April 19, 2026 20:08 View session

chatgpt-codex-connector Bot reviewed Apr 19, 2026

View reviewed changes

Comment thread browser/agent-harness/cli_anything/browser/utils/domshell_backend.py

fix(browser): reset daemon on timeout without retrying tool

5338d01

chatgpt-codex-connector Bot reviewed Apr 19, 2026

View reviewed changes

Copilot AI reviewed Apr 19, 2026

View reviewed changes

marcelsafin requested a review from Copilot April 20, 2026 21:08

Copilot started reviewing on behalf of marcelsafin April 20, 2026 21:08 View session

Copilot AI reviewed Apr 20, 2026

View reviewed changes

marcelsafin requested a review from Copilot April 20, 2026 21:37

Copilot started reviewing on behalf of marcelsafin April 20, 2026 21:37 View session

Copilot AI reviewed Apr 20, 2026

View reviewed changes

Conversation

marcelsafin commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of Change

For New Software CLIs (in-repo)

For New Software CLIs (standalone repo)

For Existing CLI Modifications

General Checklist

Test Results

Uh oh!

marcelsafin commented Apr 19, 2026

Uh oh!

marcelsafin left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

marcelsafin commented Apr 19, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

marcelsafin commented Apr 19, 2026

Uh oh!

chatgpt-codex-connector Bot commented Apr 19, 2026

Uh oh!

marcelsafin commented Apr 19, 2026

Uh oh!

chatgpt-codex-connector Bot commented Apr 19, 2026

Uh oh!

marcelsafin commented Apr 19, 2026

Uh oh!

marcelsafin commented Apr 19, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

marcelsafin commented Apr 19, 2026

Uh oh!

marcelsafin commented Apr 19, 2026

Uh oh!

marcelsafin commented Apr 19, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

marcelsafin commented Apr 19, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

marcelsafin commented Apr 19, 2026 •

edited

Loading