Skip to content

fix(execution): apply timeout to non-browser commands#383

Open
Astro-Han wants to merge 1 commit intojackwener:mainfrom
Astro-Han:fix/non-browser-timeout
Open

fix(execution): apply timeout to non-browser commands#383
Astro-Han wants to merge 1 commit intojackwener:mainfrom
Astro-Han:fix/non-browser-timeout

Conversation

@Astro-Han
Copy link
Contributor

@Astro-Han Astro-Han commented Mar 24, 2026

Description

Non-browser commands (browser: false) ran without any timeout protection, even when the adapter explicitly set timeoutSeconds. The browser branch already wrapped execution with runWithTimeout(), but the non-browser branch called runCommand() directly.

This adds the same runWithTimeout() wrapping when a positive timeoutSeconds is configured. Also adds an optional hint to TimeoutError so non-browser timeouts show a relevant message instead of the browser-specific env var hint.

Related issue: fixes the non-browser timeout gap identified in code audit

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 🌐 New site adapter
  • 📝 Documentation
  • ♻️ Refactor
  • 🔧 CI / build / tooling

Checklist

  • I ran the checks relevant to this PR
  • I updated tests or docs if needed
  • I included output or screenshots when useful

Documentation (if adding/modifying an adapter)

  • Added doc page under docs/adapters/ (if new adapter)
  • Updated docs/adapters/index.md table (if new adapter)
  • Updated sidebar in docs/.vitepress/config.mts (if new adapter)
  • Updated README.md / README.zh-CN.md when command discoverability changed
  • Used positional args for the command's primary subject unless a named flag is clearly better
  • Normalized expected adapter failures to CliError subclasses instead of raw Error

Screenshots / Output

 PASS  src/execution.test.ts
  executeCommand — non-browser timeout
    ✓ applies timeoutSeconds to non-browser commands (13ms)
    ✓ skips timeout when timeoutSeconds is 0 (52ms)

@Astro-Han Astro-Han force-pushed the fix/non-browser-timeout branch from 43bb28e to 853542e Compare March 24, 2026 14:57
@Astro-Han Astro-Han changed the title fix: apply timeout to non-browser commands fix(execution): apply timeout to non-browser commands Mar 24, 2026
@Astro-Han Astro-Han force-pushed the fix/non-browser-timeout branch 2 times, most recently from 18f4551 to 80bd70e Compare March 24, 2026 16:49
Non-browser commands (`browser: false`) ran without any timeout
protection, even when `timeoutSeconds` was explicitly set. This wraps
the non-browser execution path with `runWithTimeout()` when the
adapter defines a positive `timeoutSeconds`.

Also adds an optional `hint` parameter to `TimeoutError` so the
non-browser path shows a relevant suggestion instead of the
browser-specific `OPENCLI_BROWSER_COMMAND_TIMEOUT` env var hint.
@Astro-Han Astro-Han force-pushed the fix/non-browser-timeout branch from 3459301 to 52512da Compare March 24, 2026 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant