Skip to content

Make sandbox class work natively in workflow with "use step" - #58

Closed
TooTallNate wants to merge 3 commits into
malte/serdefrom
workflow-serde
Closed

Make sandbox class work natively in workflow with "use step"#58
TooTallNate wants to merge 3 commits into
malte/serdefrom
workflow-serde

Conversation

@TooTallNate

@TooTallNateTooTallNate commented Feb 23, 2026

Copy link
Copy Markdown
Member

Summary

Adds "use step" annotations to all API-calling async methods in the Sandbox SDK, building on the serialization support from #72. This allows Sandbox, Command, CommandFinished, and Snapshot instances to be used directly inside "use workflow" functions without needing manual step wrappers.

Changes

  • Annotate all async API methods with "use step" in Sandbox, Command, CommandFinished, and Snapshot
  • Simplify the workflow-code-runner example to call SDK methods directly in the workflow (removing the steps/sandbox.ts wrapper)

Before (manual step wrappers required)

// steps/sandbox.tsexportasyncfunctionexecute(sandbox: Sandbox,code: string){"use step";initCredentials();awaitsandbox.writeFiles([...]);constfinished=awaitsandbox.runCommand("node",["script.js"]);return{exitCode: finished.exitCode,stdout: awaitfinished.stdout(),stderr: awaitfinished.stderr()};}// workflows/code-runner.tsconstresult=awaitexecute(sandbox,code);

After (SDK methods are steps)

// workflows/code-runner.ts — no wrapper neededawaitsandbox.writeFiles([{path: "script.js",content: Buffer.from(code)}]);constfinished=awaitsandbox.runCommand("node",["script.js"]);conststdout=awaitfinished.stdout();

Test plan

  • Verify workflow-code-runner example works end-to-end
  • Confirm step replay correctly skips already-completed SDK calls

🤖 Generated with Claude Code

@vercel

vercelBot commented Feb 23, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
sandboxErrorErrorMar 20, 2026 10:26pm
sandbox-cliReadyReadyPreview, CommentMar 20, 2026 10:26pm
sandbox-sdkReadyReadyPreview, CommentMar 20, 2026 10:26pm
sandbox-sdk-ai-exampleReadyReadyPreview, CommentMar 20, 2026 10:26pm

Request Review

@pranaygppranaygp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall this is a solid approach to making Sandbox SDK classes workflow-serializable. The lazy client pattern via ensureClient() is clean, and the "use step" annotations are comprehensive. A few things worth addressing below.

return {
sandboxId: (instance as any).sandboxId,
cmd: (instance as any).cmd,
exitCode: instance.exitCode,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fragile: accessing parent private fields via (instance as any)

This duplicates the serialization logic from Command[WORKFLOW_SERIALIZE] and accesses sandboxId and cmd via as any casts because they are private on the parent class. If those field names are ever renamed, this will silently break.

Consider reusing the parent serializer:

static[WORKFLOW_SERIALIZE](instance: CommandFinished){return{
...Command[WORKFLOW_SERIALIZE](instance),exitCode: instance.exitCode,};}

This keeps CommandFinished serialization in sync with Command automatically.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — after rebasing onto #72, CommandFinished uses protected fields and accesses them directly in its serializer. No as any casts.

instance.routes = data.routes;
instance.privateParams = data.privateParams;
return instance;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

DisposableSandbox[WORKFLOW_DESERIALIZE] duplicates Sandbox[WORKFLOW_DESERIALIZE]

This is a near-copy of Sandbox[WORKFLOW_DESERIALIZE] — the only difference is the prototype used. Consider reusing the parent to avoid drift:

static[WORKFLOW_DESERIALIZE](data: {sandbox: ConvertedSandbox;
routes: SandboxRouteData[];
privateParams: Record<string,unknown>;}): DisposableSandbox{constinstance=Sandbox[WORKFLOW_DESERIALIZE](data);Object.setPrototypeOf(instance,DisposableSandbox.prototype);returninstanceasDisposableSandbox;}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — in #72's implementation, DisposableSandbox doesn't have its own WORKFLOW_SERIALIZE/WORKFLOW_DESERIALIZE at all. No duplication.

for await (const log of client.getLogs({
sandboxId: this.sandboxId,
cmdId: this.cmd.id,
signal: opts?.signal,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Signal from concurrent callers is silently ignored

When stdout() and stderr() are called concurrently, only the signal from the first call is wired into getLogs(). The second caller's signal is ignored — it will wait for the first call to finish even if its own signal is aborted.

This was pre-existing behavior (the old code via this.logs() had the same issue), but worth noting now that this is explicitly step-compatible and more likely to be called in workflow contexts where cancellation semantics matter.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a pre-existing issue in the output caching logic from #72 — not introduced or changed by this PR (which only adds "use step" annotations). Leaving for a separate follow-up if needed.

* @see {@link Command.stdout}, {@link Command.stderr}, and {@link Command.output}
* to access output as a string.
*/
logs(opts?: { signal?: AbortSignal }) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: The error message says to use output(), stdout(), or stderr() instead — but these methods internally call getCachedOutput() which calls ensureClient() and then client.getLogs(). So the real distinction is that logs() requires a pre-existing client while the other methods can lazily reconstruct one. The error message is correct in its guidance but slightly misleading about why — it's not that logs() is inherently non-step-compatible, it's that it's synchronous and can't call the async ensureClient(). Maybe worth a small clarification:

"Cannot call logs() on a deserialized Command. " +
"logs() is synchronous and cannot lazily reconstruct the API client. " +
"Use output(), stdout(), or stderr() instead."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — in #72's implementation, logs() uses the lazy get client() getter which auto-creates the client from global credentials. There's no special error message for deserialized instances.

Comment threadpackages/vercel-sandbox/package.json Outdated
"license": "Apache-2.0",
"dependencies": {
"@vercel/oidc": "3.2.0",
"@workflow/serde": "^4.1.0-beta.2",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor: @workflow/serde is pinned to a beta pre-release (^4.1.0-beta.2). With the ^ range, this will accept any >=4.1.0-beta.2 <5.0.0 including future betas and the eventual stable release. Is that intentional? If the stable API is expected to be compatible, this is fine. If the beta API might still change, consider pinning more tightly (e.g., ~4.1.0-beta.2 or exact).

@pranaygppranaygpMar 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

human: I think this is fine. we won't break serve in 4.x and we're going to 5.x soon on workflow

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved per above — staying with ^ range since serde won't break in 4.x.

Annotate all API-calling async methods in Sandbox, Command, CommandFinished,
and Snapshot with "use step" directives, enabling the Workflow DevKit runtime
to treat each method invocation as a durable step boundary.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
pranaygpand others added 2 commits March 20, 2026 15:22
* origin/malte/serde:
add workflow-code-runner example app
refactor(sdk): build @vercel/sandbox with tsdown dual outputs (#84)
Remove the steps/sandbox.ts wrapper — since all Sandbox SDK methods
now have "use step" built in, the workflow can call them directly
without intermediate step functions.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@TooTallNate

Copy link
Copy Markdown
MemberAuthor

Superseded by #109, which re-implements this approach against the current tootallnate/serde branch with the additional fix of bundle: false in tsdown and the async ensureClient() pattern to keep APIClient out of the workflow bundle's module scope.

pranaygp pushed a commit that referenced this pull request Mar 27, 2026
…ibility (#109)
## Summary
Makes `@vercel/sandbox` fully compatible with the Workflow DevKit
compiler so that `Sandbox`, `Command`, and `CommandFinished` instances
can be used directly inside `"use workflow"` functions — no wrapper step
functions needed.
Supersedes #58.
## Problem
When `@vercel/sandbox` is imported in a workflow context, the workflow
builder tries to bundle it into the workflow VM bundle (because it has
`WORKFLOW_SERIALIZE`/`WORKFLOW_DESERIALIZE` on its classes). This fails
because:
1. The SDK's public methods use Node.js APIs (`fs`, `stream`, `zlib`,
`undici`, etc.) which are forbidden in the workflow VM
2. The compiled `dist/index.js` was a single bundled file that hoisted
all Node.js imports to the top, making them impossible to tree-shake
3. The `Sandbox` and `Command` classes had a sync `client` getter that
directly referenced `APIClient`, pulling the entire HTTP client stack
into the module scope
## Solution
### 1. `"use step"` annotations on all public async methods
Added `"use step"` to all 22 public async methods across `Sandbox` (14),
`Command` (5), and `Snapshot` (3). The SWC plugin strips these method
bodies in workflow mode, replacing them with durable step proxies. This
eliminates all Node.js API references from the workflow bundle.
### 2. Async `ensureClient()` replaces sync `client` getter
The sync `get client()` getter directly referenced `APIClient`, which
pulls in `undici`, `zlib`, `tar-stream`, `jsonlines`, etc. Replaced
with:
```typescript
private async ensureClient(): Promise<APIClient> {
"use step";
if (this._client) return this._client;
const credentials = getSandboxCredentials();
this._client = new APIClient({ ... });
return this._client;
}
```
Since `ensureClient()` is itself `"use step"`, its body (including `new
APIClient(...)`) gets stripped in workflow mode. All instance methods
now call `const client = await this.ensureClient();` instead of
`this.client`.
### 3. `bundle: false` in tsdown config
Changed from single-file bundling to per-file output. This keeps Node.js
imports local to the files that use them, so after the SWC plugin strips
step method bodies, the now-unused Node.js imports can be eliminated by
esbuild's tree-shaking.
### 4. Workflow-code-runner example updated
- Removed `serverExternalPackages: ["@vercel/sandbox"]` from
`next.config.ts` (no longer needed)
- Updated `workflow` dependency to use a tarball that includes the
inline class serialization registration fix (vercel/workflow#1480)
## Testing
- `pnpm build` succeeds for the full monorepo (all 8 tasks)
- The `workflow-code-runner` example app builds successfully with all
workflow routes generated
- 22 `"use step"` directives survive compilation in both ESM and CJS
dist output
## Related
- vercel/workflow#1480 — Inline class serialization registration (fixes
SWC import resolution for 3rd-party packages)
- vercel/workflow#1481 — Build-time warning when
`serverExternalPackages` hides workflow-enabled packages
- vercel/workflow#1144 — CJS detection for serde symbols (pending
review)
- #58 — Previous attempt (superseded by this PR)
---------
Signed-off-by: Peter Wielander <mittgfu@gmail.com>
Co-authored-by: Peter Wielander <mittgfu@gmail.com>
Sign up for freeto 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.

2 participants

@TooTallNate@pranaygp
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Make sandbox class work natively in workflow with "use step" by TooTallNate · Pull Request #58 · vercel/sandbox · GitHub
Skip to content

Make sandbox class work natively in workflow with "use step" - #58

Closed
TooTallNate wants to merge 3 commits into
malte/serdefrom
workflow-serde
Closed

Make sandbox class work natively in workflow with "use step"#58
TooTallNate wants to merge 3 commits into
malte/serdefrom
workflow-serde

Conversation

@TooTallNate

@TooTallNateTooTallNate commented Feb 23, 2026

Copy link
Copy Markdown
Member

Summary

Adds "use step" annotations to all API-calling async methods in the Sandbox SDK, building on the serialization support from #72. This allows Sandbox, Command, CommandFinished, and Snapshot instances to be used directly inside "use workflow" functions without needing manual step wrappers.

Changes

  • Annotate all async API methods with "use step" in Sandbox, Command, CommandFinished, and Snapshot
  • Simplify the workflow-code-runner example to call SDK methods directly in the workflow (removing the steps/sandbox.ts wrapper)

Before (manual step wrappers required)

// steps/sandbox.tsexportasyncfunctionexecute(sandbox: Sandbox,code: string){"use step";initCredentials();awaitsandbox.writeFiles([...]);constfinished=awaitsandbox.runCommand("node",["script.js"]);return{exitCode: finished.exitCode,stdout: awaitfinished.stdout(),stderr: awaitfinished.stderr()};}// workflows/code-runner.tsconstresult=awaitexecute(sandbox,code);

After (SDK methods are steps)

// workflows/code-runner.ts — no wrapper neededawaitsandbox.writeFiles([{path: "script.js",content: Buffer.from(code)}]);constfinished=awaitsandbox.runCommand("node",["script.js"]);conststdout=awaitfinished.stdout();

Test plan

  • Verify workflow-code-runner example works end-to-end
  • Confirm step replay correctly skips already-completed SDK calls

🤖 Generated with Claude Code

@vercel

vercelBot commented Feb 23, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
sandboxErrorErrorMar 20, 2026 10:26pm
sandbox-cliReadyReadyPreview, CommentMar 20, 2026 10:26pm
sandbox-sdkReadyReadyPreview, CommentMar 20, 2026 10:26pm
sandbox-sdk-ai-exampleReadyReadyPreview, CommentMar 20, 2026 10:26pm

Request Review

@pranaygppranaygp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall this is a solid approach to making Sandbox SDK classes workflow-serializable. The lazy client pattern via ensureClient() is clean, and the "use step" annotations are comprehensive. A few things worth addressing below.

return {
sandboxId: (instance as any).sandboxId,
cmd: (instance as any).cmd,
exitCode: instance.exitCode,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fragile: accessing parent private fields via (instance as any)

This duplicates the serialization logic from Command[WORKFLOW_SERIALIZE] and accesses sandboxId and cmd via as any casts because they are private on the parent class. If those field names are ever renamed, this will silently break.

Consider reusing the parent serializer:

static[WORKFLOW_SERIALIZE](instance: CommandFinished){return{
...Command[WORKFLOW_SERIALIZE](instance),exitCode: instance.exitCode,};}

This keeps CommandFinished serialization in sync with Command automatically.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — after rebasing onto #72, CommandFinished uses protected fields and accesses them directly in its serializer. No as any casts.

instance.routes = data.routes;
instance.privateParams = data.privateParams;
return instance;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

DisposableSandbox[WORKFLOW_DESERIALIZE] duplicates Sandbox[WORKFLOW_DESERIALIZE]

This is a near-copy of Sandbox[WORKFLOW_DESERIALIZE] — the only difference is the prototype used. Consider reusing the parent to avoid drift:

static[WORKFLOW_DESERIALIZE](data: {sandbox: ConvertedSandbox;
routes: SandboxRouteData[];
privateParams: Record<string,unknown>;}): DisposableSandbox{constinstance=Sandbox[WORKFLOW_DESERIALIZE](data);Object.setPrototypeOf(instance,DisposableSandbox.prototype);returninstanceasDisposableSandbox;}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — in #72's implementation, DisposableSandbox doesn't have its own WORKFLOW_SERIALIZE/WORKFLOW_DESERIALIZE at all. No duplication.

for await (const log of client.getLogs({
sandboxId: this.sandboxId,
cmdId: this.cmd.id,
signal: opts?.signal,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Signal from concurrent callers is silently ignored

When stdout() and stderr() are called concurrently, only the signal from the first call is wired into getLogs(). The second caller's signal is ignored — it will wait for the first call to finish even if its own signal is aborted.

This was pre-existing behavior (the old code via this.logs() had the same issue), but worth noting now that this is explicitly step-compatible and more likely to be called in workflow contexts where cancellation semantics matter.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a pre-existing issue in the output caching logic from #72 — not introduced or changed by this PR (which only adds "use step" annotations). Leaving for a separate follow-up if needed.

* @see {@link Command.stdout}, {@link Command.stderr}, and {@link Command.output}
* to access output as a string.
*/
logs(opts?: { signal?: AbortSignal }) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: The error message says to use output(), stdout(), or stderr() instead — but these methods internally call getCachedOutput() which calls ensureClient() and then client.getLogs(). So the real distinction is that logs() requires a pre-existing client while the other methods can lazily reconstruct one. The error message is correct in its guidance but slightly misleading about why — it's not that logs() is inherently non-step-compatible, it's that it's synchronous and can't call the async ensureClient(). Maybe worth a small clarification:

"Cannot call logs() on a deserialized Command. " +
"logs() is synchronous and cannot lazily reconstruct the API client. " +
"Use output(), stdout(), or stderr() instead."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — in #72's implementation, logs() uses the lazy get client() getter which auto-creates the client from global credentials. There's no special error message for deserialized instances.

Comment threadpackages/vercel-sandbox/package.json Outdated
"license": "Apache-2.0",
"dependencies": {
"@vercel/oidc": "3.2.0",
"@workflow/serde": "^4.1.0-beta.2",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor: @workflow/serde is pinned to a beta pre-release (^4.1.0-beta.2). With the ^ range, this will accept any >=4.1.0-beta.2 <5.0.0 including future betas and the eventual stable release. Is that intentional? If the stable API is expected to be compatible, this is fine. If the beta API might still change, consider pinning more tightly (e.g., ~4.1.0-beta.2 or exact).

@pranaygppranaygpMar 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

human: I think this is fine. we won't break serve in 4.x and we're going to 5.x soon on workflow

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved per above — staying with ^ range since serde won't break in 4.x.

Annotate all API-calling async methods in Sandbox, Command, CommandFinished,
and Snapshot with "use step" directives, enabling the Workflow DevKit runtime
to treat each method invocation as a durable step boundary.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
pranaygpand others added 2 commits March 20, 2026 15:22
* origin/malte/serde:
add workflow-code-runner example app
refactor(sdk): build @vercel/sandbox with tsdown dual outputs (#84)
Remove the steps/sandbox.ts wrapper — since all Sandbox SDK methods
now have "use step" built in, the workflow can call them directly
without intermediate step functions.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@TooTallNate

Copy link
Copy Markdown
MemberAuthor

Superseded by #109, which re-implements this approach against the current tootallnate/serde branch with the additional fix of bundle: false in tsdown and the async ensureClient() pattern to keep APIClient out of the workflow bundle's module scope.

pranaygp pushed a commit that referenced this pull request Mar 27, 2026
…ibility (#109)
## Summary
Makes `@vercel/sandbox` fully compatible with the Workflow DevKit
compiler so that `Sandbox`, `Command`, and `CommandFinished` instances
can be used directly inside `"use workflow"` functions — no wrapper step
functions needed.
Supersedes #58.
## Problem
When `@vercel/sandbox` is imported in a workflow context, the workflow
builder tries to bundle it into the workflow VM bundle (because it has
`WORKFLOW_SERIALIZE`/`WORKFLOW_DESERIALIZE` on its classes). This fails
because:
1. The SDK's public methods use Node.js APIs (`fs`, `stream`, `zlib`,
`undici`, etc.) which are forbidden in the workflow VM
2. The compiled `dist/index.js` was a single bundled file that hoisted
all Node.js imports to the top, making them impossible to tree-shake
3. The `Sandbox` and `Command` classes had a sync `client` getter that
directly referenced `APIClient`, pulling the entire HTTP client stack
into the module scope
## Solution
### 1. `"use step"` annotations on all public async methods
Added `"use step"` to all 22 public async methods across `Sandbox` (14),
`Command` (5), and `Snapshot` (3). The SWC plugin strips these method
bodies in workflow mode, replacing them with durable step proxies. This
eliminates all Node.js API references from the workflow bundle.
### 2. Async `ensureClient()` replaces sync `client` getter
The sync `get client()` getter directly referenced `APIClient`, which
pulls in `undici`, `zlib`, `tar-stream`, `jsonlines`, etc. Replaced
with:
```typescript
private async ensureClient(): Promise<APIClient> {
"use step";
if (this._client) return this._client;
const credentials = getSandboxCredentials();
this._client = new APIClient({ ... });
return this._client;
}
```
Since `ensureClient()` is itself `"use step"`, its body (including `new
APIClient(...)`) gets stripped in workflow mode. All instance methods
now call `const client = await this.ensureClient();` instead of
`this.client`.
### 3. `bundle: false` in tsdown config
Changed from single-file bundling to per-file output. This keeps Node.js
imports local to the files that use them, so after the SWC plugin strips
step method bodies, the now-unused Node.js imports can be eliminated by
esbuild's tree-shaking.
### 4. Workflow-code-runner example updated
- Removed `serverExternalPackages: ["@vercel/sandbox"]` from
`next.config.ts` (no longer needed)
- Updated `workflow` dependency to use a tarball that includes the
inline class serialization registration fix (vercel/workflow#1480)
## Testing
- `pnpm build` succeeds for the full monorepo (all 8 tasks)
- The `workflow-code-runner` example app builds successfully with all
workflow routes generated
- 22 `"use step"` directives survive compilation in both ESM and CJS
dist output
## Related
- vercel/workflow#1480 — Inline class serialization registration (fixes
SWC import resolution for 3rd-party packages)
- vercel/workflow#1481 — Build-time warning when
`serverExternalPackages` hides workflow-enabled packages
- vercel/workflow#1144 — CJS detection for serde symbols (pending
review)
- #58 — Previous attempt (superseded by this PR)
---------
Signed-off-by: Peter Wielander <mittgfu@gmail.com>
Co-authored-by: Peter Wielander <mittgfu@gmail.com>
Sign up for freeto 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.

2 participants

@TooTallNate@pranaygp
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Make sandbox class work natively in workflow with "use step" by TooTallNate · Pull Request #58 · vercel/sandbox · GitHub
Skip to content

Make sandbox class work natively in workflow with "use step" - #58

Closed
TooTallNate wants to merge 3 commits into
malte/serdefrom
workflow-serde
Closed

Make sandbox class work natively in workflow with "use step"#58
TooTallNate wants to merge 3 commits into
malte/serdefrom
workflow-serde

Conversation

@TooTallNate

@TooTallNateTooTallNate commented Feb 23, 2026

Copy link
Copy Markdown
Member

Summary

Adds "use step" annotations to all API-calling async methods in the Sandbox SDK, building on the serialization support from #72. This allows Sandbox, Command, CommandFinished, and Snapshot instances to be used directly inside "use workflow" functions without needing manual step wrappers.

Changes

  • Annotate all async API methods with "use step" in Sandbox, Command, CommandFinished, and Snapshot
  • Simplify the workflow-code-runner example to call SDK methods directly in the workflow (removing the steps/sandbox.ts wrapper)

Before (manual step wrappers required)

// steps/sandbox.tsexportasyncfunctionexecute(sandbox: Sandbox,code: string){"use step";initCredentials();awaitsandbox.writeFiles([...]);constfinished=awaitsandbox.runCommand("node",["script.js"]);return{exitCode: finished.exitCode,stdout: awaitfinished.stdout(),stderr: awaitfinished.stderr()};}// workflows/code-runner.tsconstresult=awaitexecute(sandbox,code);

After (SDK methods are steps)

// workflows/code-runner.ts — no wrapper neededawaitsandbox.writeFiles([{path: "script.js",content: Buffer.from(code)}]);constfinished=awaitsandbox.runCommand("node",["script.js"]);conststdout=awaitfinished.stdout();

Test plan

  • Verify workflow-code-runner example works end-to-end
  • Confirm step replay correctly skips already-completed SDK calls

🤖 Generated with Claude Code

@vercel

vercelBot commented Feb 23, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
sandboxErrorErrorMar 20, 2026 10:26pm
sandbox-cliReadyReadyPreview, CommentMar 20, 2026 10:26pm
sandbox-sdkReadyReadyPreview, CommentMar 20, 2026 10:26pm
sandbox-sdk-ai-exampleReadyReadyPreview, CommentMar 20, 2026 10:26pm

Request Review

@pranaygppranaygp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall this is a solid approach to making Sandbox SDK classes workflow-serializable. The lazy client pattern via ensureClient() is clean, and the "use step" annotations are comprehensive. A few things worth addressing below.

return {
sandboxId: (instance as any).sandboxId,
cmd: (instance as any).cmd,
exitCode: instance.exitCode,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fragile: accessing parent private fields via (instance as any)

This duplicates the serialization logic from Command[WORKFLOW_SERIALIZE] and accesses sandboxId and cmd via as any casts because they are private on the parent class. If those field names are ever renamed, this will silently break.

Consider reusing the parent serializer:

static[WORKFLOW_SERIALIZE](instance: CommandFinished){return{
...Command[WORKFLOW_SERIALIZE](instance),exitCode: instance.exitCode,};}

This keeps CommandFinished serialization in sync with Command automatically.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — after rebasing onto #72, CommandFinished uses protected fields and accesses them directly in its serializer. No as any casts.

instance.routes = data.routes;
instance.privateParams = data.privateParams;
return instance;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

DisposableSandbox[WORKFLOW_DESERIALIZE] duplicates Sandbox[WORKFLOW_DESERIALIZE]

This is a near-copy of Sandbox[WORKFLOW_DESERIALIZE] — the only difference is the prototype used. Consider reusing the parent to avoid drift:

static[WORKFLOW_DESERIALIZE](data: {sandbox: ConvertedSandbox;
routes: SandboxRouteData[];
privateParams: Record<string,unknown>;}): DisposableSandbox{constinstance=Sandbox[WORKFLOW_DESERIALIZE](data);Object.setPrototypeOf(instance,DisposableSandbox.prototype);returninstanceasDisposableSandbox;}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — in #72's implementation, DisposableSandbox doesn't have its own WORKFLOW_SERIALIZE/WORKFLOW_DESERIALIZE at all. No duplication.

for await (const log of client.getLogs({
sandboxId: this.sandboxId,
cmdId: this.cmd.id,
signal: opts?.signal,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Signal from concurrent callers is silently ignored

When stdout() and stderr() are called concurrently, only the signal from the first call is wired into getLogs(). The second caller's signal is ignored — it will wait for the first call to finish even if its own signal is aborted.

This was pre-existing behavior (the old code via this.logs() had the same issue), but worth noting now that this is explicitly step-compatible and more likely to be called in workflow contexts where cancellation semantics matter.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a pre-existing issue in the output caching logic from #72 — not introduced or changed by this PR (which only adds "use step" annotations). Leaving for a separate follow-up if needed.

* @see {@link Command.stdout}, {@link Command.stderr}, and {@link Command.output}
* to access output as a string.
*/
logs(opts?: { signal?: AbortSignal }) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: The error message says to use output(), stdout(), or stderr() instead — but these methods internally call getCachedOutput() which calls ensureClient() and then client.getLogs(). So the real distinction is that logs() requires a pre-existing client while the other methods can lazily reconstruct one. The error message is correct in its guidance but slightly misleading about why — it's not that logs() is inherently non-step-compatible, it's that it's synchronous and can't call the async ensureClient(). Maybe worth a small clarification:

"Cannot call logs() on a deserialized Command. " +
"logs() is synchronous and cannot lazily reconstruct the API client. " +
"Use output(), stdout(), or stderr() instead."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — in #72's implementation, logs() uses the lazy get client() getter which auto-creates the client from global credentials. There's no special error message for deserialized instances.

Comment threadpackages/vercel-sandbox/package.json Outdated
"license": "Apache-2.0",
"dependencies": {
"@vercel/oidc": "3.2.0",
"@workflow/serde": "^4.1.0-beta.2",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor: @workflow/serde is pinned to a beta pre-release (^4.1.0-beta.2). With the ^ range, this will accept any >=4.1.0-beta.2 <5.0.0 including future betas and the eventual stable release. Is that intentional? If the stable API is expected to be compatible, this is fine. If the beta API might still change, consider pinning more tightly (e.g., ~4.1.0-beta.2 or exact).

@pranaygppranaygpMar 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

human: I think this is fine. we won't break serve in 4.x and we're going to 5.x soon on workflow

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved per above — staying with ^ range since serde won't break in 4.x.

Annotate all API-calling async methods in Sandbox, Command, CommandFinished,
and Snapshot with "use step" directives, enabling the Workflow DevKit runtime
to treat each method invocation as a durable step boundary.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
pranaygpand others added 2 commits March 20, 2026 15:22
* origin/malte/serde:
add workflow-code-runner example app
refactor(sdk): build @vercel/sandbox with tsdown dual outputs (#84)
Remove the steps/sandbox.ts wrapper — since all Sandbox SDK methods
now have "use step" built in, the workflow can call them directly
without intermediate step functions.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@TooTallNate

Copy link
Copy Markdown
MemberAuthor

Superseded by #109, which re-implements this approach against the current tootallnate/serde branch with the additional fix of bundle: false in tsdown and the async ensureClient() pattern to keep APIClient out of the workflow bundle's module scope.

pranaygp pushed a commit that referenced this pull request Mar 27, 2026
…ibility (#109)
## Summary
Makes `@vercel/sandbox` fully compatible with the Workflow DevKit
compiler so that `Sandbox`, `Command`, and `CommandFinished` instances
can be used directly inside `"use workflow"` functions — no wrapper step
functions needed.
Supersedes #58.
## Problem
When `@vercel/sandbox` is imported in a workflow context, the workflow
builder tries to bundle it into the workflow VM bundle (because it has
`WORKFLOW_SERIALIZE`/`WORKFLOW_DESERIALIZE` on its classes). This fails
because:
1. The SDK's public methods use Node.js APIs (`fs`, `stream`, `zlib`,
`undici`, etc.) which are forbidden in the workflow VM
2. The compiled `dist/index.js` was a single bundled file that hoisted
all Node.js imports to the top, making them impossible to tree-shake
3. The `Sandbox` and `Command` classes had a sync `client` getter that
directly referenced `APIClient`, pulling the entire HTTP client stack
into the module scope
## Solution
### 1. `"use step"` annotations on all public async methods
Added `"use step"` to all 22 public async methods across `Sandbox` (14),
`Command` (5), and `Snapshot` (3). The SWC plugin strips these method
bodies in workflow mode, replacing them with durable step proxies. This
eliminates all Node.js API references from the workflow bundle.
### 2. Async `ensureClient()` replaces sync `client` getter
The sync `get client()` getter directly referenced `APIClient`, which
pulls in `undici`, `zlib`, `tar-stream`, `jsonlines`, etc. Replaced
with:
```typescript
private async ensureClient(): Promise<APIClient> {
"use step";
if (this._client) return this._client;
const credentials = getSandboxCredentials();
this._client = new APIClient({ ... });
return this._client;
}
```
Since `ensureClient()` is itself `"use step"`, its body (including `new
APIClient(...)`) gets stripped in workflow mode. All instance methods
now call `const client = await this.ensureClient();` instead of
`this.client`.
### 3. `bundle: false` in tsdown config
Changed from single-file bundling to per-file output. This keeps Node.js
imports local to the files that use them, so after the SWC plugin strips
step method bodies, the now-unused Node.js imports can be eliminated by
esbuild's tree-shaking.
### 4. Workflow-code-runner example updated
- Removed `serverExternalPackages: ["@vercel/sandbox"]` from
`next.config.ts` (no longer needed)
- Updated `workflow` dependency to use a tarball that includes the
inline class serialization registration fix (vercel/workflow#1480)
## Testing
- `pnpm build` succeeds for the full monorepo (all 8 tasks)
- The `workflow-code-runner` example app builds successfully with all
workflow routes generated
- 22 `"use step"` directives survive compilation in both ESM and CJS
dist output
## Related
- vercel/workflow#1480 — Inline class serialization registration (fixes
SWC import resolution for 3rd-party packages)
- vercel/workflow#1481 — Build-time warning when
`serverExternalPackages` hides workflow-enabled packages
- vercel/workflow#1144 — CJS detection for serde symbols (pending
review)
- #58 — Previous attempt (superseded by this PR)
---------
Signed-off-by: Peter Wielander <mittgfu@gmail.com>
Co-authored-by: Peter Wielander <mittgfu@gmail.com>
Sign up for freeto 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.

2 participants

@TooTallNate@pranaygp
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Make sandbox class work natively in workflow with "use step" by TooTallNate · Pull Request #58 · vercel/sandbox · GitHub
Skip to content

Make sandbox class work natively in workflow with "use step" - #58

Closed
TooTallNate wants to merge 3 commits into
malte/serdefrom
workflow-serde
Closed

Make sandbox class work natively in workflow with "use step"#58
TooTallNate wants to merge 3 commits into
malte/serdefrom
workflow-serde

Conversation

@TooTallNate

@TooTallNateTooTallNate commented Feb 23, 2026

Copy link
Copy Markdown
Member

Summary

Adds "use step" annotations to all API-calling async methods in the Sandbox SDK, building on the serialization support from #72. This allows Sandbox, Command, CommandFinished, and Snapshot instances to be used directly inside "use workflow" functions without needing manual step wrappers.

Changes

  • Annotate all async API methods with "use step" in Sandbox, Command, CommandFinished, and Snapshot
  • Simplify the workflow-code-runner example to call SDK methods directly in the workflow (removing the steps/sandbox.ts wrapper)

Before (manual step wrappers required)

// steps/sandbox.tsexportasyncfunctionexecute(sandbox: Sandbox,code: string){"use step";initCredentials();awaitsandbox.writeFiles([...]);constfinished=awaitsandbox.runCommand("node",["script.js"]);return{exitCode: finished.exitCode,stdout: awaitfinished.stdout(),stderr: awaitfinished.stderr()};}// workflows/code-runner.tsconstresult=awaitexecute(sandbox,code);

After (SDK methods are steps)

// workflows/code-runner.ts — no wrapper neededawaitsandbox.writeFiles([{path: "script.js",content: Buffer.from(code)}]);constfinished=awaitsandbox.runCommand("node",["script.js"]);conststdout=awaitfinished.stdout();

Test plan

  • Verify workflow-code-runner example works end-to-end
  • Confirm step replay correctly skips already-completed SDK calls

🤖 Generated with Claude Code

@vercel

vercelBot commented Feb 23, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
sandboxErrorErrorMar 20, 2026 10:26pm
sandbox-cliReadyReadyPreview, CommentMar 20, 2026 10:26pm
sandbox-sdkReadyReadyPreview, CommentMar 20, 2026 10:26pm
sandbox-sdk-ai-exampleReadyReadyPreview, CommentMar 20, 2026 10:26pm

Request Review

@pranaygppranaygp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall this is a solid approach to making Sandbox SDK classes workflow-serializable. The lazy client pattern via ensureClient() is clean, and the "use step" annotations are comprehensive. A few things worth addressing below.

return {
sandboxId: (instance as any).sandboxId,
cmd: (instance as any).cmd,
exitCode: instance.exitCode,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fragile: accessing parent private fields via (instance as any)

This duplicates the serialization logic from Command[WORKFLOW_SERIALIZE] and accesses sandboxId and cmd via as any casts because they are private on the parent class. If those field names are ever renamed, this will silently break.

Consider reusing the parent serializer:

static[WORKFLOW_SERIALIZE](instance: CommandFinished){return{
...Command[WORKFLOW_SERIALIZE](instance),exitCode: instance.exitCode,};}

This keeps CommandFinished serialization in sync with Command automatically.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — after rebasing onto #72, CommandFinished uses protected fields and accesses them directly in its serializer. No as any casts.

instance.routes = data.routes;
instance.privateParams = data.privateParams;
return instance;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

DisposableSandbox[WORKFLOW_DESERIALIZE] duplicates Sandbox[WORKFLOW_DESERIALIZE]

This is a near-copy of Sandbox[WORKFLOW_DESERIALIZE] — the only difference is the prototype used. Consider reusing the parent to avoid drift:

static[WORKFLOW_DESERIALIZE](data: {sandbox: ConvertedSandbox;
routes: SandboxRouteData[];
privateParams: Record<string,unknown>;}): DisposableSandbox{constinstance=Sandbox[WORKFLOW_DESERIALIZE](data);Object.setPrototypeOf(instance,DisposableSandbox.prototype);returninstanceasDisposableSandbox;}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — in #72's implementation, DisposableSandbox doesn't have its own WORKFLOW_SERIALIZE/WORKFLOW_DESERIALIZE at all. No duplication.

for await (const log of client.getLogs({
sandboxId: this.sandboxId,
cmdId: this.cmd.id,
signal: opts?.signal,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Signal from concurrent callers is silently ignored

When stdout() and stderr() are called concurrently, only the signal from the first call is wired into getLogs(). The second caller's signal is ignored — it will wait for the first call to finish even if its own signal is aborted.

This was pre-existing behavior (the old code via this.logs() had the same issue), but worth noting now that this is explicitly step-compatible and more likely to be called in workflow contexts where cancellation semantics matter.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a pre-existing issue in the output caching logic from #72 — not introduced or changed by this PR (which only adds "use step" annotations). Leaving for a separate follow-up if needed.

* @see {@link Command.stdout}, {@link Command.stderr}, and {@link Command.output}
* to access output as a string.
*/
logs(opts?: { signal?: AbortSignal }) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: The error message says to use output(), stdout(), or stderr() instead — but these methods internally call getCachedOutput() which calls ensureClient() and then client.getLogs(). So the real distinction is that logs() requires a pre-existing client while the other methods can lazily reconstruct one. The error message is correct in its guidance but slightly misleading about why — it's not that logs() is inherently non-step-compatible, it's that it's synchronous and can't call the async ensureClient(). Maybe worth a small clarification:

"Cannot call logs() on a deserialized Command. " +
"logs() is synchronous and cannot lazily reconstruct the API client. " +
"Use output(), stdout(), or stderr() instead."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — in #72's implementation, logs() uses the lazy get client() getter which auto-creates the client from global credentials. There's no special error message for deserialized instances.

Comment threadpackages/vercel-sandbox/package.json Outdated
"license": "Apache-2.0",
"dependencies": {
"@vercel/oidc": "3.2.0",
"@workflow/serde": "^4.1.0-beta.2",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor: @workflow/serde is pinned to a beta pre-release (^4.1.0-beta.2). With the ^ range, this will accept any >=4.1.0-beta.2 <5.0.0 including future betas and the eventual stable release. Is that intentional? If the stable API is expected to be compatible, this is fine. If the beta API might still change, consider pinning more tightly (e.g., ~4.1.0-beta.2 or exact).

@pranaygppranaygpMar 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

human: I think this is fine. we won't break serve in 4.x and we're going to 5.x soon on workflow

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved per above — staying with ^ range since serde won't break in 4.x.

Annotate all API-calling async methods in Sandbox, Command, CommandFinished,
and Snapshot with "use step" directives, enabling the Workflow DevKit runtime
to treat each method invocation as a durable step boundary.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
pranaygpand others added 2 commits March 20, 2026 15:22
* origin/malte/serde:
add workflow-code-runner example app
refactor(sdk): build @vercel/sandbox with tsdown dual outputs (#84)
Remove the steps/sandbox.ts wrapper — since all Sandbox SDK methods
now have "use step" built in, the workflow can call them directly
without intermediate step functions.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@TooTallNate

Copy link
Copy Markdown
MemberAuthor

Superseded by #109, which re-implements this approach against the current tootallnate/serde branch with the additional fix of bundle: false in tsdown and the async ensureClient() pattern to keep APIClient out of the workflow bundle's module scope.

pranaygp pushed a commit that referenced this pull request Mar 27, 2026
…ibility (#109)
## Summary
Makes `@vercel/sandbox` fully compatible with the Workflow DevKit
compiler so that `Sandbox`, `Command`, and `CommandFinished` instances
can be used directly inside `"use workflow"` functions — no wrapper step
functions needed.
Supersedes #58.
## Problem
When `@vercel/sandbox` is imported in a workflow context, the workflow
builder tries to bundle it into the workflow VM bundle (because it has
`WORKFLOW_SERIALIZE`/`WORKFLOW_DESERIALIZE` on its classes). This fails
because:
1. The SDK's public methods use Node.js APIs (`fs`, `stream`, `zlib`,
`undici`, etc.) which are forbidden in the workflow VM
2. The compiled `dist/index.js` was a single bundled file that hoisted
all Node.js imports to the top, making them impossible to tree-shake
3. The `Sandbox` and `Command` classes had a sync `client` getter that
directly referenced `APIClient`, pulling the entire HTTP client stack
into the module scope
## Solution
### 1. `"use step"` annotations on all public async methods
Added `"use step"` to all 22 public async methods across `Sandbox` (14),
`Command` (5), and `Snapshot` (3). The SWC plugin strips these method
bodies in workflow mode, replacing them with durable step proxies. This
eliminates all Node.js API references from the workflow bundle.
### 2. Async `ensureClient()` replaces sync `client` getter
The sync `get client()` getter directly referenced `APIClient`, which
pulls in `undici`, `zlib`, `tar-stream`, `jsonlines`, etc. Replaced
with:
```typescript
private async ensureClient(): Promise<APIClient> {
"use step";
if (this._client) return this._client;
const credentials = getSandboxCredentials();
this._client = new APIClient({ ... });
return this._client;
}
```
Since `ensureClient()` is itself `"use step"`, its body (including `new
APIClient(...)`) gets stripped in workflow mode. All instance methods
now call `const client = await this.ensureClient();` instead of
`this.client`.
### 3. `bundle: false` in tsdown config
Changed from single-file bundling to per-file output. This keeps Node.js
imports local to the files that use them, so after the SWC plugin strips
step method bodies, the now-unused Node.js imports can be eliminated by
esbuild's tree-shaking.
### 4. Workflow-code-runner example updated
- Removed `serverExternalPackages: ["@vercel/sandbox"]` from
`next.config.ts` (no longer needed)
- Updated `workflow` dependency to use a tarball that includes the
inline class serialization registration fix (vercel/workflow#1480)
## Testing
- `pnpm build` succeeds for the full monorepo (all 8 tasks)
- The `workflow-code-runner` example app builds successfully with all
workflow routes generated
- 22 `"use step"` directives survive compilation in both ESM and CJS
dist output
## Related
- vercel/workflow#1480 — Inline class serialization registration (fixes
SWC import resolution for 3rd-party packages)
- vercel/workflow#1481 — Build-time warning when
`serverExternalPackages` hides workflow-enabled packages
- vercel/workflow#1144 — CJS detection for serde symbols (pending
review)
- #58 — Previous attempt (superseded by this PR)
---------
Signed-off-by: Peter Wielander <mittgfu@gmail.com>
Co-authored-by: Peter Wielander <mittgfu@gmail.com>
Sign up for freeto 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.

2 participants

@TooTallNate@pranaygp
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Make sandbox class work natively in workflow with "use step" by TooTallNate · Pull Request #58 · vercel/sandbox · GitHub
Skip to content

Make sandbox class work natively in workflow with "use step" - #58

Closed
TooTallNate wants to merge 3 commits into
malte/serdefrom
workflow-serde
Closed

Make sandbox class work natively in workflow with "use step"#58
TooTallNate wants to merge 3 commits into
malte/serdefrom
workflow-serde

Conversation

@TooTallNate

@TooTallNateTooTallNate commented Feb 23, 2026

Copy link
Copy Markdown
Member

Summary

Adds "use step" annotations to all API-calling async methods in the Sandbox SDK, building on the serialization support from #72. This allows Sandbox, Command, CommandFinished, and Snapshot instances to be used directly inside "use workflow" functions without needing manual step wrappers.

Changes

  • Annotate all async API methods with "use step" in Sandbox, Command, CommandFinished, and Snapshot
  • Simplify the workflow-code-runner example to call SDK methods directly in the workflow (removing the steps/sandbox.ts wrapper)

Before (manual step wrappers required)

// steps/sandbox.tsexportasyncfunctionexecute(sandbox: Sandbox,code: string){"use step";initCredentials();awaitsandbox.writeFiles([...]);constfinished=awaitsandbox.runCommand("node",["script.js"]);return{exitCode: finished.exitCode,stdout: awaitfinished.stdout(),stderr: awaitfinished.stderr()};}// workflows/code-runner.tsconstresult=awaitexecute(sandbox,code);

After (SDK methods are steps)

// workflows/code-runner.ts — no wrapper neededawaitsandbox.writeFiles([{path: "script.js",content: Buffer.from(code)}]);constfinished=awaitsandbox.runCommand("node",["script.js"]);conststdout=awaitfinished.stdout();

Test plan

  • Verify workflow-code-runner example works end-to-end
  • Confirm step replay correctly skips already-completed SDK calls

🤖 Generated with Claude Code

@vercel

vercelBot commented Feb 23, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
sandboxErrorErrorMar 20, 2026 10:26pm
sandbox-cliReadyReadyPreview, CommentMar 20, 2026 10:26pm
sandbox-sdkReadyReadyPreview, CommentMar 20, 2026 10:26pm
sandbox-sdk-ai-exampleReadyReadyPreview, CommentMar 20, 2026 10:26pm

Request Review

@pranaygppranaygp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall this is a solid approach to making Sandbox SDK classes workflow-serializable. The lazy client pattern via ensureClient() is clean, and the "use step" annotations are comprehensive. A few things worth addressing below.

return {
sandboxId: (instance as any).sandboxId,
cmd: (instance as any).cmd,
exitCode: instance.exitCode,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fragile: accessing parent private fields via (instance as any)

This duplicates the serialization logic from Command[WORKFLOW_SERIALIZE] and accesses sandboxId and cmd via as any casts because they are private on the parent class. If those field names are ever renamed, this will silently break.

Consider reusing the parent serializer:

static[WORKFLOW_SERIALIZE](instance: CommandFinished){return{
...Command[WORKFLOW_SERIALIZE](instance),exitCode: instance.exitCode,};}

This keeps CommandFinished serialization in sync with Command automatically.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — after rebasing onto #72, CommandFinished uses protected fields and accesses them directly in its serializer. No as any casts.

instance.routes = data.routes;
instance.privateParams = data.privateParams;
return instance;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

DisposableSandbox[WORKFLOW_DESERIALIZE] duplicates Sandbox[WORKFLOW_DESERIALIZE]

This is a near-copy of Sandbox[WORKFLOW_DESERIALIZE] — the only difference is the prototype used. Consider reusing the parent to avoid drift:

static[WORKFLOW_DESERIALIZE](data: {sandbox: ConvertedSandbox;
routes: SandboxRouteData[];
privateParams: Record<string,unknown>;}): DisposableSandbox{constinstance=Sandbox[WORKFLOW_DESERIALIZE](data);Object.setPrototypeOf(instance,DisposableSandbox.prototype);returninstanceasDisposableSandbox;}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — in #72's implementation, DisposableSandbox doesn't have its own WORKFLOW_SERIALIZE/WORKFLOW_DESERIALIZE at all. No duplication.

for await (const log of client.getLogs({
sandboxId: this.sandboxId,
cmdId: this.cmd.id,
signal: opts?.signal,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Signal from concurrent callers is silently ignored

When stdout() and stderr() are called concurrently, only the signal from the first call is wired into getLogs(). The second caller's signal is ignored — it will wait for the first call to finish even if its own signal is aborted.

This was pre-existing behavior (the old code via this.logs() had the same issue), but worth noting now that this is explicitly step-compatible and more likely to be called in workflow contexts where cancellation semantics matter.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a pre-existing issue in the output caching logic from #72 — not introduced or changed by this PR (which only adds "use step" annotations). Leaving for a separate follow-up if needed.

* @see {@link Command.stdout}, {@link Command.stderr}, and {@link Command.output}
* to access output as a string.
*/
logs(opts?: { signal?: AbortSignal }) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: The error message says to use output(), stdout(), or stderr() instead — but these methods internally call getCachedOutput() which calls ensureClient() and then client.getLogs(). So the real distinction is that logs() requires a pre-existing client while the other methods can lazily reconstruct one. The error message is correct in its guidance but slightly misleading about why — it's not that logs() is inherently non-step-compatible, it's that it's synchronous and can't call the async ensureClient(). Maybe worth a small clarification:

"Cannot call logs() on a deserialized Command. " +
"logs() is synchronous and cannot lazily reconstruct the API client. " +
"Use output(), stdout(), or stderr() instead."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — in #72's implementation, logs() uses the lazy get client() getter which auto-creates the client from global credentials. There's no special error message for deserialized instances.

Comment threadpackages/vercel-sandbox/package.json Outdated
"license": "Apache-2.0",
"dependencies": {
"@vercel/oidc": "3.2.0",
"@workflow/serde": "^4.1.0-beta.2",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor: @workflow/serde is pinned to a beta pre-release (^4.1.0-beta.2). With the ^ range, this will accept any >=4.1.0-beta.2 <5.0.0 including future betas and the eventual stable release. Is that intentional? If the stable API is expected to be compatible, this is fine. If the beta API might still change, consider pinning more tightly (e.g., ~4.1.0-beta.2 or exact).

@pranaygppranaygpMar 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

human: I think this is fine. we won't break serve in 4.x and we're going to 5.x soon on workflow

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved per above — staying with ^ range since serde won't break in 4.x.

Annotate all API-calling async methods in Sandbox, Command, CommandFinished,
and Snapshot with "use step" directives, enabling the Workflow DevKit runtime
to treat each method invocation as a durable step boundary.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
pranaygpand others added 2 commits March 20, 2026 15:22
* origin/malte/serde:
add workflow-code-runner example app
refactor(sdk): build @vercel/sandbox with tsdown dual outputs (#84)
Remove the steps/sandbox.ts wrapper — since all Sandbox SDK methods
now have "use step" built in, the workflow can call them directly
without intermediate step functions.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@TooTallNate

Copy link
Copy Markdown
MemberAuthor

Superseded by #109, which re-implements this approach against the current tootallnate/serde branch with the additional fix of bundle: false in tsdown and the async ensureClient() pattern to keep APIClient out of the workflow bundle's module scope.

pranaygp pushed a commit that referenced this pull request Mar 27, 2026
…ibility (#109)
## Summary
Makes `@vercel/sandbox` fully compatible with the Workflow DevKit
compiler so that `Sandbox`, `Command`, and `CommandFinished` instances
can be used directly inside `"use workflow"` functions — no wrapper step
functions needed.
Supersedes #58.
## Problem
When `@vercel/sandbox` is imported in a workflow context, the workflow
builder tries to bundle it into the workflow VM bundle (because it has
`WORKFLOW_SERIALIZE`/`WORKFLOW_DESERIALIZE` on its classes). This fails
because:
1. The SDK's public methods use Node.js APIs (`fs`, `stream`, `zlib`,
`undici`, etc.) which are forbidden in the workflow VM
2. The compiled `dist/index.js` was a single bundled file that hoisted
all Node.js imports to the top, making them impossible to tree-shake
3. The `Sandbox` and `Command` classes had a sync `client` getter that
directly referenced `APIClient`, pulling the entire HTTP client stack
into the module scope
## Solution
### 1. `"use step"` annotations on all public async methods
Added `"use step"` to all 22 public async methods across `Sandbox` (14),
`Command` (5), and `Snapshot` (3). The SWC plugin strips these method
bodies in workflow mode, replacing them with durable step proxies. This
eliminates all Node.js API references from the workflow bundle.
### 2. Async `ensureClient()` replaces sync `client` getter
The sync `get client()` getter directly referenced `APIClient`, which
pulls in `undici`, `zlib`, `tar-stream`, `jsonlines`, etc. Replaced
with:
```typescript
private async ensureClient(): Promise<APIClient> {
"use step";
if (this._client) return this._client;
const credentials = getSandboxCredentials();
this._client = new APIClient({ ... });
return this._client;
}
```
Since `ensureClient()` is itself `"use step"`, its body (including `new
APIClient(...)`) gets stripped in workflow mode. All instance methods
now call `const client = await this.ensureClient();` instead of
`this.client`.
### 3. `bundle: false` in tsdown config
Changed from single-file bundling to per-file output. This keeps Node.js
imports local to the files that use them, so after the SWC plugin strips
step method bodies, the now-unused Node.js imports can be eliminated by
esbuild's tree-shaking.
### 4. Workflow-code-runner example updated
- Removed `serverExternalPackages: ["@vercel/sandbox"]` from
`next.config.ts` (no longer needed)
- Updated `workflow` dependency to use a tarball that includes the
inline class serialization registration fix (vercel/workflow#1480)
## Testing
- `pnpm build` succeeds for the full monorepo (all 8 tasks)
- The `workflow-code-runner` example app builds successfully with all
workflow routes generated
- 22 `"use step"` directives survive compilation in both ESM and CJS
dist output
## Related
- vercel/workflow#1480 — Inline class serialization registration (fixes
SWC import resolution for 3rd-party packages)
- vercel/workflow#1481 — Build-time warning when
`serverExternalPackages` hides workflow-enabled packages
- vercel/workflow#1144 — CJS detection for serde symbols (pending
review)
- #58 — Previous attempt (superseded by this PR)
---------
Signed-off-by: Peter Wielander <mittgfu@gmail.com>
Co-authored-by: Peter Wielander <mittgfu@gmail.com>
Sign up for freeto 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.

2 participants

@TooTallNate@pranaygp
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Make sandbox class work natively in workflow with "use step" by TooTallNate · Pull Request #58 · vercel/sandbox · GitHub
Skip to content

Make sandbox class work natively in workflow with "use step" - #58

Closed
TooTallNate wants to merge 3 commits into
malte/serdefrom
workflow-serde
Closed

Make sandbox class work natively in workflow with "use step"#58
TooTallNate wants to merge 3 commits into
malte/serdefrom
workflow-serde

Conversation

@TooTallNate

@TooTallNateTooTallNate commented Feb 23, 2026

Copy link
Copy Markdown
Member

Summary

Adds "use step" annotations to all API-calling async methods in the Sandbox SDK, building on the serialization support from #72. This allows Sandbox, Command, CommandFinished, and Snapshot instances to be used directly inside "use workflow" functions without needing manual step wrappers.

Changes

  • Annotate all async API methods with "use step" in Sandbox, Command, CommandFinished, and Snapshot
  • Simplify the workflow-code-runner example to call SDK methods directly in the workflow (removing the steps/sandbox.ts wrapper)

Before (manual step wrappers required)

// steps/sandbox.tsexportasyncfunctionexecute(sandbox: Sandbox,code: string){"use step";initCredentials();awaitsandbox.writeFiles([...]);constfinished=awaitsandbox.runCommand("node",["script.js"]);return{exitCode: finished.exitCode,stdout: awaitfinished.stdout(),stderr: awaitfinished.stderr()};}// workflows/code-runner.tsconstresult=awaitexecute(sandbox,code);

After (SDK methods are steps)

// workflows/code-runner.ts — no wrapper neededawaitsandbox.writeFiles([{path: "script.js",content: Buffer.from(code)}]);constfinished=awaitsandbox.runCommand("node",["script.js"]);conststdout=awaitfinished.stdout();

Test plan

  • Verify workflow-code-runner example works end-to-end
  • Confirm step replay correctly skips already-completed SDK calls

🤖 Generated with Claude Code

@vercel

vercelBot commented Feb 23, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
sandboxErrorErrorMar 20, 2026 10:26pm
sandbox-cliReadyReadyPreview, CommentMar 20, 2026 10:26pm
sandbox-sdkReadyReadyPreview, CommentMar 20, 2026 10:26pm
sandbox-sdk-ai-exampleReadyReadyPreview, CommentMar 20, 2026 10:26pm

Request Review

@pranaygppranaygp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall this is a solid approach to making Sandbox SDK classes workflow-serializable. The lazy client pattern via ensureClient() is clean, and the "use step" annotations are comprehensive. A few things worth addressing below.

return {
sandboxId: (instance as any).sandboxId,
cmd: (instance as any).cmd,
exitCode: instance.exitCode,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fragile: accessing parent private fields via (instance as any)

This duplicates the serialization logic from Command[WORKFLOW_SERIALIZE] and accesses sandboxId and cmd via as any casts because they are private on the parent class. If those field names are ever renamed, this will silently break.

Consider reusing the parent serializer:

static[WORKFLOW_SERIALIZE](instance: CommandFinished){return{
...Command[WORKFLOW_SERIALIZE](instance),exitCode: instance.exitCode,};}

This keeps CommandFinished serialization in sync with Command automatically.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — after rebasing onto #72, CommandFinished uses protected fields and accesses them directly in its serializer. No as any casts.

instance.routes = data.routes;
instance.privateParams = data.privateParams;
return instance;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

DisposableSandbox[WORKFLOW_DESERIALIZE] duplicates Sandbox[WORKFLOW_DESERIALIZE]

This is a near-copy of Sandbox[WORKFLOW_DESERIALIZE] — the only difference is the prototype used. Consider reusing the parent to avoid drift:

static[WORKFLOW_DESERIALIZE](data: {sandbox: ConvertedSandbox;
routes: SandboxRouteData[];
privateParams: Record<string,unknown>;}): DisposableSandbox{constinstance=Sandbox[WORKFLOW_DESERIALIZE](data);Object.setPrototypeOf(instance,DisposableSandbox.prototype);returninstanceasDisposableSandbox;}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — in #72's implementation, DisposableSandbox doesn't have its own WORKFLOW_SERIALIZE/WORKFLOW_DESERIALIZE at all. No duplication.

for await (const log of client.getLogs({
sandboxId: this.sandboxId,
cmdId: this.cmd.id,
signal: opts?.signal,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Signal from concurrent callers is silently ignored

When stdout() and stderr() are called concurrently, only the signal from the first call is wired into getLogs(). The second caller's signal is ignored — it will wait for the first call to finish even if its own signal is aborted.

This was pre-existing behavior (the old code via this.logs() had the same issue), but worth noting now that this is explicitly step-compatible and more likely to be called in workflow contexts where cancellation semantics matter.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a pre-existing issue in the output caching logic from #72 — not introduced or changed by this PR (which only adds "use step" annotations). Leaving for a separate follow-up if needed.

* @see {@link Command.stdout}, {@link Command.stderr}, and {@link Command.output}
* to access output as a string.
*/
logs(opts?: { signal?: AbortSignal }) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: The error message says to use output(), stdout(), or stderr() instead — but these methods internally call getCachedOutput() which calls ensureClient() and then client.getLogs(). So the real distinction is that logs() requires a pre-existing client while the other methods can lazily reconstruct one. The error message is correct in its guidance but slightly misleading about why — it's not that logs() is inherently non-step-compatible, it's that it's synchronous and can't call the async ensureClient(). Maybe worth a small clarification:

"Cannot call logs() on a deserialized Command. " +
"logs() is synchronous and cannot lazily reconstruct the API client. " +
"Use output(), stdout(), or stderr() instead."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — in #72's implementation, logs() uses the lazy get client() getter which auto-creates the client from global credentials. There's no special error message for deserialized instances.

Comment threadpackages/vercel-sandbox/package.json Outdated
"license": "Apache-2.0",
"dependencies": {
"@vercel/oidc": "3.2.0",
"@workflow/serde": "^4.1.0-beta.2",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor: @workflow/serde is pinned to a beta pre-release (^4.1.0-beta.2). With the ^ range, this will accept any >=4.1.0-beta.2 <5.0.0 including future betas and the eventual stable release. Is that intentional? If the stable API is expected to be compatible, this is fine. If the beta API might still change, consider pinning more tightly (e.g., ~4.1.0-beta.2 or exact).

@pranaygppranaygpMar 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

human: I think this is fine. we won't break serve in 4.x and we're going to 5.x soon on workflow

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved per above — staying with ^ range since serde won't break in 4.x.

Annotate all API-calling async methods in Sandbox, Command, CommandFinished,
and Snapshot with "use step" directives, enabling the Workflow DevKit runtime
to treat each method invocation as a durable step boundary.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
pranaygpand others added 2 commits March 20, 2026 15:22
* origin/malte/serde:
add workflow-code-runner example app
refactor(sdk): build @vercel/sandbox with tsdown dual outputs (#84)
Remove the steps/sandbox.ts wrapper — since all Sandbox SDK methods
now have "use step" built in, the workflow can call them directly
without intermediate step functions.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@TooTallNate

Copy link
Copy Markdown
MemberAuthor

Superseded by #109, which re-implements this approach against the current tootallnate/serde branch with the additional fix of bundle: false in tsdown and the async ensureClient() pattern to keep APIClient out of the workflow bundle's module scope.

pranaygp pushed a commit that referenced this pull request Mar 27, 2026
…ibility (#109)
## Summary
Makes `@vercel/sandbox` fully compatible with the Workflow DevKit
compiler so that `Sandbox`, `Command`, and `CommandFinished` instances
can be used directly inside `"use workflow"` functions — no wrapper step
functions needed.
Supersedes #58.
## Problem
When `@vercel/sandbox` is imported in a workflow context, the workflow
builder tries to bundle it into the workflow VM bundle (because it has
`WORKFLOW_SERIALIZE`/`WORKFLOW_DESERIALIZE` on its classes). This fails
because:
1. The SDK's public methods use Node.js APIs (`fs`, `stream`, `zlib`,
`undici`, etc.) which are forbidden in the workflow VM
2. The compiled `dist/index.js` was a single bundled file that hoisted
all Node.js imports to the top, making them impossible to tree-shake
3. The `Sandbox` and `Command` classes had a sync `client` getter that
directly referenced `APIClient`, pulling the entire HTTP client stack
into the module scope
## Solution
### 1. `"use step"` annotations on all public async methods
Added `"use step"` to all 22 public async methods across `Sandbox` (14),
`Command` (5), and `Snapshot` (3). The SWC plugin strips these method
bodies in workflow mode, replacing them with durable step proxies. This
eliminates all Node.js API references from the workflow bundle.
### 2. Async `ensureClient()` replaces sync `client` getter
The sync `get client()` getter directly referenced `APIClient`, which
pulls in `undici`, `zlib`, `tar-stream`, `jsonlines`, etc. Replaced
with:
```typescript
private async ensureClient(): Promise<APIClient> {
"use step";
if (this._client) return this._client;
const credentials = getSandboxCredentials();
this._client = new APIClient({ ... });
return this._client;
}
```
Since `ensureClient()` is itself `"use step"`, its body (including `new
APIClient(...)`) gets stripped in workflow mode. All instance methods
now call `const client = await this.ensureClient();` instead of
`this.client`.
### 3. `bundle: false` in tsdown config
Changed from single-file bundling to per-file output. This keeps Node.js
imports local to the files that use them, so after the SWC plugin strips
step method bodies, the now-unused Node.js imports can be eliminated by
esbuild's tree-shaking.
### 4. Workflow-code-runner example updated
- Removed `serverExternalPackages: ["@vercel/sandbox"]` from
`next.config.ts` (no longer needed)
- Updated `workflow` dependency to use a tarball that includes the
inline class serialization registration fix (vercel/workflow#1480)
## Testing
- `pnpm build` succeeds for the full monorepo (all 8 tasks)
- The `workflow-code-runner` example app builds successfully with all
workflow routes generated
- 22 `"use step"` directives survive compilation in both ESM and CJS
dist output
## Related
- vercel/workflow#1480 — Inline class serialization registration (fixes
SWC import resolution for 3rd-party packages)
- vercel/workflow#1481 — Build-time warning when
`serverExternalPackages` hides workflow-enabled packages
- vercel/workflow#1144 — CJS detection for serde symbols (pending
review)
- #58 — Previous attempt (superseded by this PR)
---------
Signed-off-by: Peter Wielander <mittgfu@gmail.com>
Co-authored-by: Peter Wielander <mittgfu@gmail.com>
Sign up for freeto 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.

2 participants

@TooTallNate@pranaygp
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Make sandbox class work natively in workflow with "use step" by TooTallNate · Pull Request #58 · vercel/sandbox · GitHub
Skip to content

Make sandbox class work natively in workflow with "use step" - #58

Closed
TooTallNate wants to merge 3 commits into
malte/serdefrom
workflow-serde
Closed

Make sandbox class work natively in workflow with "use step"#58
TooTallNate wants to merge 3 commits into
malte/serdefrom
workflow-serde

Conversation

@TooTallNate

@TooTallNateTooTallNate commented Feb 23, 2026

Copy link
Copy Markdown
Member

Summary

Adds "use step" annotations to all API-calling async methods in the Sandbox SDK, building on the serialization support from #72. This allows Sandbox, Command, CommandFinished, and Snapshot instances to be used directly inside "use workflow" functions without needing manual step wrappers.

Changes

  • Annotate all async API methods with "use step" in Sandbox, Command, CommandFinished, and Snapshot
  • Simplify the workflow-code-runner example to call SDK methods directly in the workflow (removing the steps/sandbox.ts wrapper)

Before (manual step wrappers required)

// steps/sandbox.tsexportasyncfunctionexecute(sandbox: Sandbox,code: string){"use step";initCredentials();awaitsandbox.writeFiles([...]);constfinished=awaitsandbox.runCommand("node",["script.js"]);return{exitCode: finished.exitCode,stdout: awaitfinished.stdout(),stderr: awaitfinished.stderr()};}// workflows/code-runner.tsconstresult=awaitexecute(sandbox,code);

After (SDK methods are steps)

// workflows/code-runner.ts — no wrapper neededawaitsandbox.writeFiles([{path: "script.js",content: Buffer.from(code)}]);constfinished=awaitsandbox.runCommand("node",["script.js"]);conststdout=awaitfinished.stdout();

Test plan

  • Verify workflow-code-runner example works end-to-end
  • Confirm step replay correctly skips already-completed SDK calls

🤖 Generated with Claude Code

@vercel

vercelBot commented Feb 23, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
sandboxErrorErrorMar 20, 2026 10:26pm
sandbox-cliReadyReadyPreview, CommentMar 20, 2026 10:26pm
sandbox-sdkReadyReadyPreview, CommentMar 20, 2026 10:26pm
sandbox-sdk-ai-exampleReadyReadyPreview, CommentMar 20, 2026 10:26pm

Request Review

@pranaygppranaygp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall this is a solid approach to making Sandbox SDK classes workflow-serializable. The lazy client pattern via ensureClient() is clean, and the "use step" annotations are comprehensive. A few things worth addressing below.

return {
sandboxId: (instance as any).sandboxId,
cmd: (instance as any).cmd,
exitCode: instance.exitCode,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fragile: accessing parent private fields via (instance as any)

This duplicates the serialization logic from Command[WORKFLOW_SERIALIZE] and accesses sandboxId and cmd via as any casts because they are private on the parent class. If those field names are ever renamed, this will silently break.

Consider reusing the parent serializer:

static[WORKFLOW_SERIALIZE](instance: CommandFinished){return{
...Command[WORKFLOW_SERIALIZE](instance),exitCode: instance.exitCode,};}

This keeps CommandFinished serialization in sync with Command automatically.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — after rebasing onto #72, CommandFinished uses protected fields and accesses them directly in its serializer. No as any casts.

instance.routes = data.routes;
instance.privateParams = data.privateParams;
return instance;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

DisposableSandbox[WORKFLOW_DESERIALIZE] duplicates Sandbox[WORKFLOW_DESERIALIZE]

This is a near-copy of Sandbox[WORKFLOW_DESERIALIZE] — the only difference is the prototype used. Consider reusing the parent to avoid drift:

static[WORKFLOW_DESERIALIZE](data: {sandbox: ConvertedSandbox;
routes: SandboxRouteData[];
privateParams: Record<string,unknown>;}): DisposableSandbox{constinstance=Sandbox[WORKFLOW_DESERIALIZE](data);Object.setPrototypeOf(instance,DisposableSandbox.prototype);returninstanceasDisposableSandbox;}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — in #72's implementation, DisposableSandbox doesn't have its own WORKFLOW_SERIALIZE/WORKFLOW_DESERIALIZE at all. No duplication.

for await (const log of client.getLogs({
sandboxId: this.sandboxId,
cmdId: this.cmd.id,
signal: opts?.signal,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Signal from concurrent callers is silently ignored

When stdout() and stderr() are called concurrently, only the signal from the first call is wired into getLogs(). The second caller's signal is ignored — it will wait for the first call to finish even if its own signal is aborted.

This was pre-existing behavior (the old code via this.logs() had the same issue), but worth noting now that this is explicitly step-compatible and more likely to be called in workflow contexts where cancellation semantics matter.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a pre-existing issue in the output caching logic from #72 — not introduced or changed by this PR (which only adds "use step" annotations). Leaving for a separate follow-up if needed.

* @see {@link Command.stdout}, {@link Command.stderr}, and {@link Command.output}
* to access output as a string.
*/
logs(opts?: { signal?: AbortSignal }) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: The error message says to use output(), stdout(), or stderr() instead — but these methods internally call getCachedOutput() which calls ensureClient() and then client.getLogs(). So the real distinction is that logs() requires a pre-existing client while the other methods can lazily reconstruct one. The error message is correct in its guidance but slightly misleading about why — it's not that logs() is inherently non-step-compatible, it's that it's synchronous and can't call the async ensureClient(). Maybe worth a small clarification:

"Cannot call logs() on a deserialized Command. " +
"logs() is synchronous and cannot lazily reconstruct the API client. " +
"Use output(), stdout(), or stderr() instead."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — in #72's implementation, logs() uses the lazy get client() getter which auto-creates the client from global credentials. There's no special error message for deserialized instances.

Comment threadpackages/vercel-sandbox/package.json Outdated
"license": "Apache-2.0",
"dependencies": {
"@vercel/oidc": "3.2.0",
"@workflow/serde": "^4.1.0-beta.2",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor: @workflow/serde is pinned to a beta pre-release (^4.1.0-beta.2). With the ^ range, this will accept any >=4.1.0-beta.2 <5.0.0 including future betas and the eventual stable release. Is that intentional? If the stable API is expected to be compatible, this is fine. If the beta API might still change, consider pinning more tightly (e.g., ~4.1.0-beta.2 or exact).

@pranaygppranaygpMar 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

human: I think this is fine. we won't break serve in 4.x and we're going to 5.x soon on workflow

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved per above — staying with ^ range since serde won't break in 4.x.

Annotate all API-calling async methods in Sandbox, Command, CommandFinished,
and Snapshot with "use step" directives, enabling the Workflow DevKit runtime
to treat each method invocation as a durable step boundary.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
pranaygpand others added 2 commits March 20, 2026 15:22
* origin/malte/serde:
add workflow-code-runner example app
refactor(sdk): build @vercel/sandbox with tsdown dual outputs (#84)
Remove the steps/sandbox.ts wrapper — since all Sandbox SDK methods
now have "use step" built in, the workflow can call them directly
without intermediate step functions.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@TooTallNate

Copy link
Copy Markdown
MemberAuthor

Superseded by #109, which re-implements this approach against the current tootallnate/serde branch with the additional fix of bundle: false in tsdown and the async ensureClient() pattern to keep APIClient out of the workflow bundle's module scope.

pranaygp pushed a commit that referenced this pull request Mar 27, 2026
…ibility (#109)
## Summary
Makes `@vercel/sandbox` fully compatible with the Workflow DevKit
compiler so that `Sandbox`, `Command`, and `CommandFinished` instances
can be used directly inside `"use workflow"` functions — no wrapper step
functions needed.
Supersedes #58.
## Problem
When `@vercel/sandbox` is imported in a workflow context, the workflow
builder tries to bundle it into the workflow VM bundle (because it has
`WORKFLOW_SERIALIZE`/`WORKFLOW_DESERIALIZE` on its classes). This fails
because:
1. The SDK's public methods use Node.js APIs (`fs`, `stream`, `zlib`,
`undici`, etc.) which are forbidden in the workflow VM
2. The compiled `dist/index.js` was a single bundled file that hoisted
all Node.js imports to the top, making them impossible to tree-shake
3. The `Sandbox` and `Command` classes had a sync `client` getter that
directly referenced `APIClient`, pulling the entire HTTP client stack
into the module scope
## Solution
### 1. `"use step"` annotations on all public async methods
Added `"use step"` to all 22 public async methods across `Sandbox` (14),
`Command` (5), and `Snapshot` (3). The SWC plugin strips these method
bodies in workflow mode, replacing them with durable step proxies. This
eliminates all Node.js API references from the workflow bundle.
### 2. Async `ensureClient()` replaces sync `client` getter
The sync `get client()` getter directly referenced `APIClient`, which
pulls in `undici`, `zlib`, `tar-stream`, `jsonlines`, etc. Replaced
with:
```typescript
private async ensureClient(): Promise<APIClient> {
"use step";
if (this._client) return this._client;
const credentials = getSandboxCredentials();
this._client = new APIClient({ ... });
return this._client;
}
```
Since `ensureClient()` is itself `"use step"`, its body (including `new
APIClient(...)`) gets stripped in workflow mode. All instance methods
now call `const client = await this.ensureClient();` instead of
`this.client`.
### 3. `bundle: false` in tsdown config
Changed from single-file bundling to per-file output. This keeps Node.js
imports local to the files that use them, so after the SWC plugin strips
step method bodies, the now-unused Node.js imports can be eliminated by
esbuild's tree-shaking.
### 4. Workflow-code-runner example updated
- Removed `serverExternalPackages: ["@vercel/sandbox"]` from
`next.config.ts` (no longer needed)
- Updated `workflow` dependency to use a tarball that includes the
inline class serialization registration fix (vercel/workflow#1480)
## Testing
- `pnpm build` succeeds for the full monorepo (all 8 tasks)
- The `workflow-code-runner` example app builds successfully with all
workflow routes generated
- 22 `"use step"` directives survive compilation in both ESM and CJS
dist output
## Related
- vercel/workflow#1480 — Inline class serialization registration (fixes
SWC import resolution for 3rd-party packages)
- vercel/workflow#1481 — Build-time warning when
`serverExternalPackages` hides workflow-enabled packages
- vercel/workflow#1144 — CJS detection for serde symbols (pending
review)
- #58 — Previous attempt (superseded by this PR)
---------
Signed-off-by: Peter Wielander <mittgfu@gmail.com>
Co-authored-by: Peter Wielander <mittgfu@gmail.com>
Sign up for freeto 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.

2 participants

@TooTallNate@pranaygp
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); Make sandbox class work natively in workflow with "use step" by TooTallNate · Pull Request #58 · vercel/sandbox · GitHub
Skip to content

Make sandbox class work natively in workflow with "use step" - #58

Closed
TooTallNate wants to merge 3 commits into
malte/serdefrom
workflow-serde
Closed

Make sandbox class work natively in workflow with "use step"#58
TooTallNate wants to merge 3 commits into
malte/serdefrom
workflow-serde

Conversation

@TooTallNate

@TooTallNateTooTallNate commented Feb 23, 2026

Copy link
Copy Markdown
Member

Summary

Adds "use step" annotations to all API-calling async methods in the Sandbox SDK, building on the serialization support from #72. This allows Sandbox, Command, CommandFinished, and Snapshot instances to be used directly inside "use workflow" functions without needing manual step wrappers.

Changes

  • Annotate all async API methods with "use step" in Sandbox, Command, CommandFinished, and Snapshot
  • Simplify the workflow-code-runner example to call SDK methods directly in the workflow (removing the steps/sandbox.ts wrapper)

Before (manual step wrappers required)

// steps/sandbox.tsexportasyncfunctionexecute(sandbox: Sandbox,code: string){"use step";initCredentials();awaitsandbox.writeFiles([...]);constfinished=awaitsandbox.runCommand("node",["script.js"]);return{exitCode: finished.exitCode,stdout: awaitfinished.stdout(),stderr: awaitfinished.stderr()};}// workflows/code-runner.tsconstresult=awaitexecute(sandbox,code);

After (SDK methods are steps)

// workflows/code-runner.ts — no wrapper neededawaitsandbox.writeFiles([{path: "script.js",content: Buffer.from(code)}]);constfinished=awaitsandbox.runCommand("node",["script.js"]);conststdout=awaitfinished.stdout();

Test plan

  • Verify workflow-code-runner example works end-to-end
  • Confirm step replay correctly skips already-completed SDK calls

🤖 Generated with Claude Code

@vercel

vercelBot commented Feb 23, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
sandboxErrorErrorMar 20, 2026 10:26pm
sandbox-cliReadyReadyPreview, CommentMar 20, 2026 10:26pm
sandbox-sdkReadyReadyPreview, CommentMar 20, 2026 10:26pm
sandbox-sdk-ai-exampleReadyReadyPreview, CommentMar 20, 2026 10:26pm

Request Review

@pranaygppranaygp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall this is a solid approach to making Sandbox SDK classes workflow-serializable. The lazy client pattern via ensureClient() is clean, and the "use step" annotations are comprehensive. A few things worth addressing below.

return {
sandboxId: (instance as any).sandboxId,
cmd: (instance as any).cmd,
exitCode: instance.exitCode,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fragile: accessing parent private fields via (instance as any)

This duplicates the serialization logic from Command[WORKFLOW_SERIALIZE] and accesses sandboxId and cmd via as any casts because they are private on the parent class. If those field names are ever renamed, this will silently break.

Consider reusing the parent serializer:

static[WORKFLOW_SERIALIZE](instance: CommandFinished){return{
...Command[WORKFLOW_SERIALIZE](instance),exitCode: instance.exitCode,};}

This keeps CommandFinished serialization in sync with Command automatically.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — after rebasing onto #72, CommandFinished uses protected fields and accesses them directly in its serializer. No as any casts.

instance.routes = data.routes;
instance.privateParams = data.privateParams;
return instance;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

DisposableSandbox[WORKFLOW_DESERIALIZE] duplicates Sandbox[WORKFLOW_DESERIALIZE]

This is a near-copy of Sandbox[WORKFLOW_DESERIALIZE] — the only difference is the prototype used. Consider reusing the parent to avoid drift:

static[WORKFLOW_DESERIALIZE](data: {sandbox: ConvertedSandbox;
routes: SandboxRouteData[];
privateParams: Record<string,unknown>;}): DisposableSandbox{constinstance=Sandbox[WORKFLOW_DESERIALIZE](data);Object.setPrototypeOf(instance,DisposableSandbox.prototype);returninstanceasDisposableSandbox;}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — in #72's implementation, DisposableSandbox doesn't have its own WORKFLOW_SERIALIZE/WORKFLOW_DESERIALIZE at all. No duplication.

for await (const log of client.getLogs({
sandboxId: this.sandboxId,
cmdId: this.cmd.id,
signal: opts?.signal,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Signal from concurrent callers is silently ignored

When stdout() and stderr() are called concurrently, only the signal from the first call is wired into getLogs(). The second caller's signal is ignored — it will wait for the first call to finish even if its own signal is aborted.

This was pre-existing behavior (the old code via this.logs() had the same issue), but worth noting now that this is explicitly step-compatible and more likely to be called in workflow contexts where cancellation semantics matter.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a pre-existing issue in the output caching logic from #72 — not introduced or changed by this PR (which only adds "use step" annotations). Leaving for a separate follow-up if needed.

* @see {@link Command.stdout}, {@link Command.stderr}, and {@link Command.output}
* to access output as a string.
*/
logs(opts?: { signal?: AbortSignal }) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: The error message says to use output(), stdout(), or stderr() instead — but these methods internally call getCachedOutput() which calls ensureClient() and then client.getLogs(). So the real distinction is that logs() requires a pre-existing client while the other methods can lazily reconstruct one. The error message is correct in its guidance but slightly misleading about why — it's not that logs() is inherently non-step-compatible, it's that it's synchronous and can't call the async ensureClient(). Maybe worth a small clarification:

"Cannot call logs() on a deserialized Command. " +
"logs() is synchronous and cannot lazily reconstruct the API client. " +
"Use output(), stdout(), or stderr() instead."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved — in #72's implementation, logs() uses the lazy get client() getter which auto-creates the client from global credentials. There's no special error message for deserialized instances.

Comment threadpackages/vercel-sandbox/package.json Outdated
"license": "Apache-2.0",
"dependencies": {
"@vercel/oidc": "3.2.0",
"@workflow/serde": "^4.1.0-beta.2",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor: @workflow/serde is pinned to a beta pre-release (^4.1.0-beta.2). With the ^ range, this will accept any >=4.1.0-beta.2 <5.0.0 including future betas and the eventual stable release. Is that intentional? If the stable API is expected to be compatible, this is fine. If the beta API might still change, consider pinning more tightly (e.g., ~4.1.0-beta.2 or exact).

@pranaygppranaygpMar 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

human: I think this is fine. we won't break serve in 4.x and we're going to 5.x soon on workflow

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved per above — staying with ^ range since serde won't break in 4.x.

Annotate all API-calling async methods in Sandbox, Command, CommandFinished,
and Snapshot with "use step" directives, enabling the Workflow DevKit runtime
to treat each method invocation as a durable step boundary.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
pranaygpand others added 2 commits March 20, 2026 15:22
* origin/malte/serde:
add workflow-code-runner example app
refactor(sdk): build @vercel/sandbox with tsdown dual outputs (#84)
Remove the steps/sandbox.ts wrapper — since all Sandbox SDK methods
now have "use step" built in, the workflow can call them directly
without intermediate step functions.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@TooTallNate

Copy link
Copy Markdown
MemberAuthor

Superseded by #109, which re-implements this approach against the current tootallnate/serde branch with the additional fix of bundle: false in tsdown and the async ensureClient() pattern to keep APIClient out of the workflow bundle's module scope.

pranaygp pushed a commit that referenced this pull request Mar 27, 2026
…ibility (#109)
## Summary
Makes `@vercel/sandbox` fully compatible with the Workflow DevKit
compiler so that `Sandbox`, `Command`, and `CommandFinished` instances
can be used directly inside `"use workflow"` functions — no wrapper step
functions needed.
Supersedes #58.
## Problem
When `@vercel/sandbox` is imported in a workflow context, the workflow
builder tries to bundle it into the workflow VM bundle (because it has
`WORKFLOW_SERIALIZE`/`WORKFLOW_DESERIALIZE` on its classes). This fails
because:
1. The SDK's public methods use Node.js APIs (`fs`, `stream`, `zlib`,
`undici`, etc.) which are forbidden in the workflow VM
2. The compiled `dist/index.js` was a single bundled file that hoisted
all Node.js imports to the top, making them impossible to tree-shake
3. The `Sandbox` and `Command` classes had a sync `client` getter that
directly referenced `APIClient`, pulling the entire HTTP client stack
into the module scope
## Solution
### 1. `"use step"` annotations on all public async methods
Added `"use step"` to all 22 public async methods across `Sandbox` (14),
`Command` (5), and `Snapshot` (3). The SWC plugin strips these method
bodies in workflow mode, replacing them with durable step proxies. This
eliminates all Node.js API references from the workflow bundle.
### 2. Async `ensureClient()` replaces sync `client` getter
The sync `get client()` getter directly referenced `APIClient`, which
pulls in `undici`, `zlib`, `tar-stream`, `jsonlines`, etc. Replaced
with:
```typescript
private async ensureClient(): Promise<APIClient> {
"use step";
if (this._client) return this._client;
const credentials = getSandboxCredentials();
this._client = new APIClient({ ... });
return this._client;
}
```
Since `ensureClient()` is itself `"use step"`, its body (including `new
APIClient(...)`) gets stripped in workflow mode. All instance methods
now call `const client = await this.ensureClient();` instead of
`this.client`.
### 3. `bundle: false` in tsdown config
Changed from single-file bundling to per-file output. This keeps Node.js
imports local to the files that use them, so after the SWC plugin strips
step method bodies, the now-unused Node.js imports can be eliminated by
esbuild's tree-shaking.
### 4. Workflow-code-runner example updated
- Removed `serverExternalPackages: ["@vercel/sandbox"]` from
`next.config.ts` (no longer needed)
- Updated `workflow` dependency to use a tarball that includes the
inline class serialization registration fix (vercel/workflow#1480)
## Testing
- `pnpm build` succeeds for the full monorepo (all 8 tasks)
- The `workflow-code-runner` example app builds successfully with all
workflow routes generated
- 22 `"use step"` directives survive compilation in both ESM and CJS
dist output
## Related
- vercel/workflow#1480 — Inline class serialization registration (fixes
SWC import resolution for 3rd-party packages)
- vercel/workflow#1481 — Build-time warning when
`serverExternalPackages` hides workflow-enabled packages
- vercel/workflow#1144 — CJS detection for serde symbols (pending
review)
- #58 — Previous attempt (superseded by this PR)
---------
Signed-off-by: Peter Wielander <mittgfu@gmail.com>
Co-authored-by: Peter Wielander <mittgfu@gmail.com>
Sign up for freeto 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.

2 participants

@TooTallNate@pranaygp