Repository navigation
Add "use step" annotations and lazy client for Workflow DevKit compatibility #109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
68fba2b
Add "use step" annotations and lazy client for Workflow DevKit compat…
TooTallNate 0dade22
Move setSandboxCredentials to module scope
TooTallNate 9dcf079
Use getCredentials() instead of getSandboxCredentials() in ensureClie…
TooTallNate f6b6fdd
?
TooTallNate 157827a
Fix test: adapt to ensureClient() replacing sync client getter
TooTallNate 9fb4c80
Remove --turbopack flag from build script
TooTallNate 3fb0127
Fix non-sense
TooTallNate 2c51b43
update workflow
VaguelySerious 0e6aa0b
update workflow
VaguelySerious 38805c2
bump
VaguelySerious File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| /.next | ||
| /next-env.d.ts |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,6 @@ | ||
| import { withWorkflow } from "workflow/next"; | ||
| import type { NextConfig } from "next"; | ||
|
|
||
| const nextConfig: NextConfig = { | ||
| serverExternalPackages: ["@vercel/sandbox"], | ||
| }; | ||
| const nextConfig: NextConfig = {}; | ||
|
|
||
| export default withWorkflow(nextConfig); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| { | ||
| "outputDirectory": "./.next" | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| import { APIClient, type CommandData } from "./api-client/index.js"; | ||
| import { Signal, resolveSignal } from "./utils/resolveSignal.js"; | ||
| import { getSandboxCredentials } from "./utils/sandbox-credentials.js"; | ||
| import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from "@workflow/serde"; | ||
| import { WORKFLOW_DESERIALIZE, WORKFLOW_SERIALIZE } from '@workflow/serde'; | ||
| import { APIClient, type CommandData } from './api-client/index.js'; | ||
| import { getCredentials } from './utils/get-credentials.js'; | ||
| import { resolveSignal, type Signal } from './utils/resolveSignal.js'; | ||
|
|
||
| /** | ||
| * Cached output from a command execution. | ||
|
|
@@ -49,17 +49,17 @@ export class Command { | |
| protected _client: APIClient | null = null; | ||
|
|
||
| /** | ||
| * Lazily get or create the API client. | ||
| * If no client was provided during construction, creates one using global credentials. | ||
| * Lazily resolve credentials and construct an API client. | ||
| * @internal | ||
| */ | ||
| get client(): APIClient { | ||
| if (!this._client) { | ||
| const credentials = getSandboxCredentials(); | ||
| this._client = new APIClient({ | ||
| teamId: credentials.teamId, | ||
| token: credentials.token, | ||
| }); | ||
| } | ||
| protected async ensureClient(): Promise<APIClient> { | ||
| 'use step'; | ||
| if (this._client) return this._client; | ||
| const credentials = await getCredentials(); | ||
| this._client = new APIClient({ | ||
| teamId: credentials.teamId, | ||
| token: credentials.token, | ||
| }); | ||
| return this._client; | ||
| } | ||
|
|
||
|
|
@@ -190,7 +190,12 @@ export class Command { | |
| * to access output as a string. | ||
| */ | ||
| logs(opts?: { signal?: AbortSignal }) { | ||
| return this.client.getLogs({ | ||
| if (!this._client) { | ||
| throw new Error( | ||
| 'logs() requires an API client. Call an async method first to initialize the client.' | ||
| ); | ||
| } | ||
| return this._client.getLogs({ | ||
| sandboxId: this.sandboxId, | ||
| cmdId: this.cmd.id, | ||
| signal: opts?.signal, | ||
|
|
@@ -217,17 +222,19 @@ export class Command { | |
| * @returns A {@link CommandFinished} instance with populated exit code. | ||
| */ | ||
| async wait(params?: { signal?: AbortSignal }) { | ||
| 'use step'; | ||
| const client = await this.ensureClient(); | ||
| params?.signal?.throwIfAborted(); | ||
|
|
||
| const command = await this.client.getCommand({ | ||
| const command = await client.getCommand({ | ||
| sandboxId: this.sandboxId, | ||
| cmdId: this.cmd.id, | ||
| wait: true, | ||
| signal: params?.signal, | ||
| }); | ||
|
|
||
| return new CommandFinished({ | ||
| client: this.client, | ||
| client, | ||
| sandboxId: this.sandboxId, | ||
| cmd: command.json.command, | ||
| exitCode: command.json.command.exitCode, | ||
|
|
@@ -246,12 +253,12 @@ export class Command { | |
| if (!this.outputCache) { | ||
| this.outputCache = (async () => { | ||
| try { | ||
| let stdout = ""; | ||
| let stderr = ""; | ||
| let both = ""; | ||
| let stdout = ''; | ||
| let stderr = ''; | ||
| let both = ''; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| for await (const log of this.logs({ signal: opts?.signal })) { | ||
| both += log.data; | ||
| if (log.stream === "stdout") { | ||
| if (log.stream === 'stdout') { | ||
| stdout += log.data; | ||
| } else { | ||
| stderr += log.data; | ||
|
|
@@ -283,9 +290,10 @@ export class Command { | |
| * @returns The output of the specified stream(s) as a string. | ||
| */ | ||
| async output( | ||
| stream: "stdout" | "stderr" | "both" = "both", | ||
| stream: 'stdout' | 'stderr' | 'both' = 'both', | ||
| opts?: { signal?: AbortSignal } | ||
| ) { | ||
| 'use step'; | ||
| const cached = await this.getCachedOutput(opts); | ||
| return cached[stream]; | ||
| } | ||
|
|
@@ -301,7 +309,8 @@ export class Command { | |
| * @returns The standard output of the command. | ||
| */ | ||
| async stdout(opts?: { signal?: AbortSignal }) { | ||
| return this.output("stdout", opts); | ||
| 'use step'; | ||
| return this.output('stdout', opts); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -315,7 +324,8 @@ export class Command { | |
| * @returns The standard error output of the command. | ||
| */ | ||
| async stderr(opts?: { signal?: AbortSignal }) { | ||
| return this.output("stderr", opts); | ||
| 'use step'; | ||
| return this.output('stderr', opts); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -327,10 +337,12 @@ export class Command { | |
| * @returns Promise<void>. | ||
| */ | ||
| async kill(signal?: Signal, opts?: { abortSignal?: AbortSignal }) { | ||
| await this.client.killCommand({ | ||
| 'use step'; | ||
| const client = await this.ensureClient(); | ||
| await client.killCommand({ | ||
| sandboxId: this.sandboxId, | ||
| commandId: this.cmd.id, | ||
| signal: resolveSignal(signal ?? "SIGTERM"), | ||
| signal: resolveSignal(signal ?? 'SIGTERM'), | ||
| abortSignal: opts?.abortSignal, | ||
| }); | ||
| } | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Command.logs()now throws when_clientis unset, which is a behavior change vs the previous lazy client creation and breaks log streaming for deserializedCommandinstances (or any instance constructed without a client). If this is intentional for workflow compatibility, please document it in the JSDoc; otherwise consider an alternative API that can initialize the client asynchronously (or accept a client/fetch options) so logs can still be consumed without a prior async call.