Skip to content

fix: Enforce timeout_ms in shell and shell_command to prevent orphan processes #145

Closed

Description

@minorcell

Summary

shell / shell_command expose timeout_ms, but the value is currently passed as yield_time_ms and does not terminate execution. This can return a running session and leave a process alive without a continuation path in shell-only mode.

  • Affected code: /Users/mcell/Desktop/workspace/memo-cli/packages/tools/src/tools/shell.ts:34
  • Affected code: /Users/mcell/Desktop/workspace/memo-cli/packages/tools/src/tools/shell_command.ts:26
  • Runtime behavior source: /Users/mcell/Desktop/workspace/memo-cli/packages/tools/src/tools/exec_runtime.ts:238

Why this matters

  • API contract confusion: timeout_ms implies command timeout, not just response wait window.
  • Resource leak risk: processes continue running after tool call returns.
  • In MEMO_SHELL_TOOL_TYPE=shell|shell_command, there is no write_stdin tool to manage returned session IDs.

Repro

Call:

  • shell_command.execute({ command: 'sleep 2; echo done', login: false, timeout_ms: 100 })

Observed:

  • Returns in ~100ms with Process running with session ID ...
  • Does not fail on timeout
  • Process continues beyond timeout window

Expected

timeout_ms should enforce an execution deadline (kill process and return timeout error or explicit timeout status).

Actual

It only controls wait duration before returning intermediate output.

Suggested fix

  • Add real timeout handling in runtime (timer + signal/kill + deterministic timeout response).
  • Keep yield_time_ms as separate parameter for streaming semantics.
  • Add tests for timeout termination and shell-only mode behavior.

Requested implementer

Please let memo handle this fix.

Activity

  1. added
    bugSomething isn't working
    needs-triageIssue needs initial triage
    area:toolsBuilt-in tools and tool runtime
    area:coreCore runtime and session state
    on Feb 11, 2026
  2. minorcell commented on Feb 11, 2026

    @minorcell
    OwnerAuthor

    Fixed in commit 4ec1580.

    Changes:

    • Added runtime execution timeout support (execution_timeout_ms) in exec session manager.
    • Timeout now terminates long-running processes (SIGTERM, fallback SIGKILL) and returns deterministic timeout errors.
    • Updated shell and shell_command to map timeout_ms to real execution timeout (not only output wait window).
    • Added regression tests for timeout behavior in both shell wrappers.

    Validation:

    • pnpm test packages/tools/src/tools/shell_update_plan.test.ts
    • pnpm test packages/tools/src/tools/codex_tools.test.ts
  3. minorcell commented on Feb 11, 2026

    @minorcell
    OwnerAuthor

    Closing as fixed by 4ec1580.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:coreCore runtime and session statearea:toolsBuilt-in tools and tool runtimebugSomething isn't workingneeds-triageIssue needs initial triagepriority:p1High priority

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions