Skip to content

Conversation

@whoiskatrin
Copy link
Collaborator

  1. Persist to storage: setKeepAlive() now writes to ctx.storage.put('keepAliveEnabled', keepAlive)
  2. Restore on wakeup: Constructor now reads from storage: ctx.storage.get('keepAliveEnabled')

@changeset-bot
Copy link

changeset-bot bot commented Oct 28, 2025

🦋 Changeset detected

Latest commit: e06fb6d

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

This PR includes changesets to release 1 package
Name Type
@cloudflare/sandbox 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
Copy link

claude bot commented Oct 28, 2025

Claude Code Review

Status: Looks good with minor observations

This PR correctly implements keepAlive flag persistence across DO hibernation cycles.

Changes

  • ✅ setKeepAlive() persists to storage (sandbox.ts:143)
  • ✅ Constructor restores from storage (sandbox.ts:104)
  • ✅ E2E test validates persistence behavior

Observations

1. Pre-existing inconsistency (not introduced by this PR):
baseUrl is persisted in setBaseUrl() (line 127) but NOT restored from storage during initialization (lines 100-112). For consistency, consider restoring it alongside sandboxName, defaultSession, etc.

2. E2E test timing:
The test waits 20s for "potential DO hibernation" (line 303) but cannot guarantee hibernation actually occurred. Test passes either way, which means it validates storage persistence but may not definitively prove hibernation resilience. This is acceptable given DO hibernation is non-deterministic in test environments.

Verdict

Approve - The implementation is correct and well-tested. The observations above are minor and do not block this PR.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 28, 2025

Open in StackBlitz

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

commit: e06fb6d

@github-actions
Copy link
Contributor

github-actions bot commented Oct 28, 2025

🐳 Docker Image Published

FROM cloudflare/sandbox:0.0.0-pr-164-7d59984

Version: 0.0.0-pr-164-7d59984

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

@whoiskatrin whoiskatrin marked this pull request as draft October 29, 2025 11:30
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