Skip to content

fix(examples): enforce timeout parameter in bashkit-pi bash tool#1872

Merged
chaliy merged 1 commit into
mainfrom
fix/issue-1870-timeout
Jun 5, 2026
Merged

fix(examples): enforce timeout parameter in bashkit-pi bash tool#1872
chaliy merged 1 commit into
mainfrom
fix/issue-1870-timeout

Conversation

@chaliy
Copy link
Copy Markdown
Contributor

@chaliy chaliy commented Jun 5, 2026

Summary

  • The bash tool in examples/bashkit-pi/bashkit-extension.ts exposed a timeout parameter in its schema but never used it — executeSync was always called without any timeout bound.
  • Fix: convert params.timeout (seconds) to milliseconds and pass it as an AbortSignal via AbortSignal.timeout(ms) to executeSync, which accepts { signal } in its ExecuteOptions.
  • When no timeout is provided the behavior is unchanged (unbounded, subject to Bash instance limits).

Test plan

  • Verify the extension loads without errors: cd examples/bashkit-pi && pnpm install
  • Confirm that a command with a short timeout (e.g. timeout: 1 with sleep 5) is cancelled after ~1 s
  • Confirm that a command without a timeout still runs normally

Closes #1870

Pass params.timeout (converted from seconds to ms) as an AbortSignal to
executeSync, so per-call timeout bounds are actually enforced instead of
silently ignored.
Copilot AI review requested due to automatic review settings June 5, 2026 00:38
@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
bashkit 26daa03 Commit Preview URL

Branch Preview URL
Jun 05 2026, 12:39 AM

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the bash tool implementation in the bashkit-pi example so that the timeout parameter exposed in the tool schema is actually enforced during command execution by passing an AbortSignal into bash.executeSync.

Changes:

  • Convert params.timeout (seconds) to milliseconds.
  • Pass AbortSignal.timeout(ms) as { signal } to bash.executeSync to cancel long-running commands when requested.

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

Comment on lines +96 to +102
// Convert caller-supplied seconds to milliseconds for AbortSignal.
// If no timeout is given, execute unbounded (subject to Bash instance limits).
const signal =
params.timeout != null
? AbortSignal.timeout(params.timeout * 1000)
: undefined;
const result = bash.executeSync(params.command, { signal });
@chaliy chaliy merged commit 7aee315 into main Jun 5, 2026
17 checks passed
@chaliy chaliy deleted the fix/issue-1870-timeout branch June 5, 2026 00:51
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.

[DeepSec][BUG] Tool timeout parameter is ignored

2 participants