Skip to content

fix(mcp): stop the remote command and close its channel when run_command times out - #1293

Open
doublewhy wants to merge 1 commit into
devfrom
1290-run-command-timeout
Open

doublewhy wants to merge 1 commit into
devfrom
1290-run-command-timeout

Conversation

@doublewhy

@doublewhy doublewhy commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Summary

run_command runs through SSHConnectionManager.executeCommand with the fixed 30 s default timeout. When the timer fired, executeCommand rejected but kept no reference to the exec channel: the channel stayed open, the remote command kept running, and every chunk it wrote was still appended to the result buffers inside the MCP server (evidence and a hermetic reproduction in #1290). This PR stops the command and closes its channel when the timer fires.

Changes

mcp/aptl-mcp-common/src/ssh-manager.ts:

  • executeCommand keeps the channel. On timeout the new stopTimedOutCommand sends TERM with ssh2's ClientChannel.signal(), then close(), before the promise rejects. A channel the server opens only after the timer fired is stopped the same way as soon as it opens.
  • Late output still reaches the capture tee (ptyTee) but is no longer buffered; one collect helper holds that guard for both data handlers.
  • ssh2 throws from signal()/close() once its connection is torn down (Instance unusable after …); stopTimedOutCommand catches and logs it, so it cannot crash the server from the timer.
  • The rejection message, the 30 s default and the success path are unchanged.

tests/ssh.test.ts adds a #1290 block that drives the real executeCommand through a stub connection and an EventEmitter channel with stderr, signal() and close(). No file is added or removed and no lockfile changes, so the inventory and MCP lockfile digests are unchanged.

Done-when matrix

Tests are in the #1290 block, run by CI's MCP TypeScript tests + coverage job (npx vitest run --coverage in mcp/aptl-mcp-common; passed in Checks run 38226224239). "Stash" = dev's ssh-manager.ts restored with git stash push.

Done-when (verbatim) Change Test Without the fix
A test with a mocked channel shows that the timeout signals the command, closes the channel once, and rejects with the unchanged timeout error. ssh-manager.ts:stopTimedOutCommand, executeCommand signals the command, closes the channel and stops buffering when the timer fires stash: fails, signal never called with TERM
Output that arrives after the timeout is not buffered, and the capture tee still records it. ssh-manager.ts:executeCommand (collect) same test (tee records out, out, err) stash: fails; collect guard alone removed: fails (late chunk's toString called 2 times)
A channel that opens after the timer fired is signalled and closed. ssh-manager.ts:executeCommand (exec callback) stops a channel the server opens only after the timer fired stash: fails, signal never called with TERM
A command that finishes in time resolves as before and is neither signalled nor closed. success path, unchanged leaves a command that finishes in time alone passes on both; guards the success path

still rejects when ssh2 throws because the connection is already gone fails under stash (nothing logged) and with the try/catch removed (Instance unusable after fatal error escapes the timer).

Scope

Overlap with #1279

#1279 (#967) edits other functions in the same two files. On dev 1abe440, git merge-tree --write-tree --name-only HEAD 22afced2 for this head and #1279's head 22afced is clean (exit 0, tree 76eb78b6) and keeps both PRs' changes.

Verification

Head 083df5a, based on dev 1abe440, Node 22.23.3, npm 10.9.9.

Requirement UIDs: none (bug/maintenance).

Closes #1290

…and times out

When the 30 s timer of the one-shot exec behind run_command fired,
executeCommand rejected but kept no reference to the channel. The remote
command kept running, and every chunk it wrote was still appended to the
stdout and stderr buffers inside the MCP server.

executeCommand now keeps the channel. When the timer fires it sends the
command TERM and closes the channel, and a channel the server opens only
after that is stopped as soon as it opens. Output that arrives after the
timeout still reaches the capture tee but is no longer buffered. The
rejection message and the default timeout are unchanged. A server may
refuse the signal (OpenSSH does for a ForceCommand session), which the
close covers, and an error from ssh2 on a torn-down connection is logged
instead of escaping the timer.

Closes #1290

This branch has not been deployed

No deployments
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