Skip to content
This repository was archived by the owner on Sep 23, 2026. It is now read-only.

fix(shell): adapt timeouts for long commands - #2200

Closed
he-yufeng wants to merge 1 commit into
MoonshotAI:mainfrom
he-yufeng:fix/shell-command-timeout-hints
Closed

he-yufeng wants to merge 1 commit into
MoonshotAI:mainfrom
he-yufeng:fix/shell-command-timeout-hints

Conversation

@he-yufeng

@he-yufeng he-yufeng commented May 8, 2026 •

Copy link
Copy Markdown

Summary

  • extend the shell timeout automatically for command patterns that are commonly slow, such as git submodule cleanup, git clone/fetch, package installs, and builds
  • keep normal commands at the existing 60s default
  • preserve a larger explicit timeout when the caller already supplies one

Fixes #2195.

To verify

  • python -m uv run pytest tests\tools\test_shell_powershell.py tests\tools\test_shell_timeout_policy.py -q
  • python -m uv run ruff check src\kimi_cli\tools\shell\__init__.py tests\tools\test_shell_timeout_policy.py
  • python -m uv run ruff format src\kimi_cli\tools\shell\__init__.py tests\tools\test_shell_timeout_policy.py --check
  • python -m py_compile src\kimi_cli\tools\shell\__init__.py tests\tools\test_shell_timeout_policy.py
  • git diff --check

Open in Devin Review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6257ebe753

ℹ️ 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".

normalized = " ".join(command.split())
for pattern, suggested_timeout in LONG_RUNNING_COMMAND_TIMEOUTS:
if pattern.search(normalized):
return min(max(timeout, suggested_timeout), max_timeout)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Respect caller-provided shorter timeout values

Do not unconditionally raise the timeout for matched commands: _effective_timeout currently returns max(timeout, suggested_timeout), so Shell(timeout=5) on commands like npm install will actually run for 180s. This changes the semantics of the timeout parameter from an explicit limit to a minimum and can cause callers that rely on fast-fail behavior to block for minutes unexpectedly; consider only applying adaptive extension when the caller kept the default timeout.

Useful? React with 👍 / 👎.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

@he-yufeng
he-yufeng force-pushed the fix/shell-command-timeout-hints branch from 6257ebe to bbadafd Compare May 11, 2026 02:56

Copy link
Copy Markdown
Author

Rebased this onto the latest main and resolved the shell-tool conflict by keeping both recent behaviors: Windows command preprocessing still runs before execution, and long-running commands now compute the adaptive timeout from the same command string that is approved and executed.

Validation on head bbadafd9:

  • python -m uv run pytest tests\tools\test_shell_timeout_policy.py -q (5 passed)
  • python -m uv run pytest tests\tools\test_shell_bash.py tests\tools\test_shell_timeout_policy.py -q (5 passed, 24 skipped)
  • python -m uv run ruff check src\kimi_cli\tools\shell\__init__.py tests\tools\test_shell_timeout_policy.py
  • python -m py_compile src\kimi_cli\tools\shell\__init__.py tests\tools\test_shell_timeout_policy.py
  • git diff --check upstream/main..HEAD

Pytest still reports a local .pytest_cache permission warning in this checkout, but the tests completed successfully.

@he-yufeng

Copy link
Copy Markdown
Author

Gentle bump on this one. The long-command timeout adaptation still applies cleanly on current main, and it is a small self-contained change. Happy to rebase if it is useful to pick up.

@he-yufeng

Copy link
Copy Markdown
Author

Third and final nudge. src/kimi_cli/tools/shell/__init__.py is untouched on current main, so long-running commands still hit the fixed timeout and the adaptive policy here still applies. Glad to adjust the approach if you would rather see this handled somewhere else.

@he-yufeng

Copy link
Copy Markdown
Author

The repo README announces the wind-down into kimi-code and this has had three nudges with no review. Closing rather than pinging a fourth time.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Shell] Command timeout is rigid (60s) and not configurable or adaptive

1 participant