Add timeouts to file and git operations to prevent indefinite hangs

[whoiskatrin]

No description provided.

[changeset-bot]

🦋 Changeset detected

Latest commit: 19a595f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@cloudflare/sandbox	Patch
@repo/sandbox-container	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[claude]

Claude Code Review

Summary: Good fix for issue #166. The timeout implementation is sound and well-documented.

Issues Found:

1. Inconsistent timeout for move() operation (file-service.ts:609)

   • move() uses 30s timeout but rename() (which it wraps) uses 10s
   • The outer 30s timeout will never trigger since rename() times out at 10s first
   • Fix: Either use matching timeouts or document why they differ

2. Missing timeout validation

   • No validation that timeouts are positive numbers
   • Session.ts accepts any timeoutMs value without bounds checking
   • Consider: Add validation in Session.waitForExitCode() or at API boundaries

3. Test coverage gaps

   • Tests verify timeout values are passed but don't test timeout behavior
   • No test cases for operations that actually time out
   • Consider: Add at least one test with a deliberately slow operation to verify timeout mechanism works

Minor observations:

• ProcessService change is correct - properly unifies the options object pattern
• All test updates correctly reflect the new API signature
• Documentation in FileService class comment is excellent
• Changeset is comprehensive and well-written

Verdict: Approve with suggestion to address timeout consistency in move() operation.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/sandbox-sdk/@cloudflare/sandbox@171

commit: 19a595f

[github-actions]

🐳 Docker Image Published

FROM cloudflare/sandbox:0.0.0-pr-171-515452d

Version: 0.0.0-pr-171-515452d

You can use this Docker image with the preview package from this PR.

[threepointone]

I'll wait for this to land before rebasing and landing #172

[ghostwriternr]

Closing in favour of #185