Conversation
Combine execute, runtimes, and fetch into piston.ts package, support for passing api key to client, setup proxy in order to attach api key to piston requests
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
apps:browser-extension
apps:discord-bot
apps:website
dependencies
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
WalkthroughMigrate execution and runtime code from legacy Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser Client / Extension
participant WebsiteAPI as Website /api/piston
participant PistonUpstream as Upstream Piston API
Browser->>WebsiteAPI: POST /api/piston/<route> (with Origin)
WebsiteAPI->>WebsiteAPI: matchOrigin(origin, ALLOWED_ORIGIN)
alt origin allowed
WebsiteAPI->>PistonUpstream: Forward request (Authorization: PISTON_API_KEY)
PistonUpstream-->>WebsiteAPI: Response (status, body)
WebsiteAPI-->>Browser: Proxy response (status, body)
else origin denied
WebsiteAPI-->>Browser: 403 Forbidden
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
packages/piston.ts/src/arguments.ts (1)
1-1: Nit: tighten the@ts-expect-errordescription.The comment text is a bit informal for a shared package. Consider a more descriptive note so contributors understand what specifically is broken.
💬 Suggested wording
-// `@ts-expect-error` - Lexure has crap typings +// `@ts-expect-error` - Lexure does not ship usable type declarations🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/piston.ts/src/arguments.ts` at line 1, Replace the informal file header comment "// `@ts-expect-error` - Lexure has crap typings" with a concise, professional `@ts-expect-error` note that explains which Lexure typings are incorrect or missing, what symbols/types in this file are affected (so future contributors can understand the reason), and optionally reference a tracking GitHub issue or PR number and a TODO to remove the suppression once the upstream types are fixed; locate the existing string to update and ensure the new comment remains immediately above the offending import/line so the `@ts-expect-error` still applies..env/.env.example (1)
4-6:ALLOWED_ORIGINis not strictly required — it has a server-side default of'*'.Based on the website
env.ts(z.string().default('*')),ALLOWED_ORIGINfalls back to allowing all origins when unset. Marking the whole section "Required" overstates the constraint and may confuse contributors. Consider splitting the comment or annotating the individual variables.💬 Suggested wording
-# Piston (Required) -PISTON_API_KEY= -ALLOWED_ORIGIN= +# Piston +PISTON_API_KEY= # Required +ALLOWED_ORIGIN= # Optional — defaults to '*' (allow all origins)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env/.env.example around lines 4 - 6, The "Piston (Required)" header incorrectly implies ALLOWED_ORIGIN is required; update the .env example to mark PISTON_API_KEY as required but annotate ALLOWED_ORIGIN as optional (or show a separate "Optional" line) and mention its server-side default '*' (see env.ts z.string().default('*')) so contributors know it falls back to allowing all origins when unset; keep variable names PISTON_API_KEY and ALLOWED_ORIGIN unchanged.apps/discord-bot/src/commands/evaluate.ts (1)
50-63: Consider deduplicating the response-mapping logic.Both branches apply the same
.slice(0, 25).map(…)transformation. A minor simplification:♻️ Optional refactor
async autocomplete(interaction: AutocompleteInteraction) { const runtime = interaction.options.getString('runtime'); - - if (runtime) { - const runtimes = await piston.runtimes.search(runtime); - return interaction.respond( - runtimes.slice(0, 25).map((r) => ({ name: r.name, value: r.id })), - ); - } else { - const runtimes = await piston.runtimes(); - return interaction.respond( - runtimes.slice(0, 25).map((r) => ({ name: r.name, value: r.id })), - ); - } + const runtimes = runtime + ? await piston.runtimes.search(runtime) + : await piston.runtimes(); + return interaction.respond( + runtimes.slice(0, 25).map((r) => ({ name: r.name, value: r.id })), + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/discord-bot/src/commands/evaluate.ts` around lines 50 - 63, The autocomplete method duplicates the same slice-and-map response logic in both branches; refactor autocomplete to compute runtimes via either piston.runtimes.search(runtime) when runtime is present or piston.runtimes() otherwise, then apply the shared transformation runtimes.slice(0, 25).map(r => ({ name: r.name, value: r.id })) and call interaction.respond once; update the function (autocomplete) so only the runtime-fetch differs and the mapping/responding is centralized, referencing piston.runtimes.search, piston.runtimes, and interaction.respond.apps/browser-extension/src/services/piston.ts (1)
4-6: Code works correctly, but consider aligning with codebase patterns for consistency.The concern about a missing path separator is not actually an issue:
env.VITE_PUBLIC_WEBSITE_URLis a URL object (viaz.url().transform((v) => new URL(v))), and when stringified in a template literal, the URL constructor normalises it to include a trailing slash. Therefore${new URL('https://example.com')}api/pistoncorrectly produceshttps://example.com/api/piston.However, the codebase already uses a more explicit and consistent pattern elsewhere (e.g. in content-script files):
new URL('api/piston', env.VITE_PUBLIC_WEBSITE_URL)Consider adopting this pattern here and in
posthog.ts(which uses the same approach) for consistency and clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/browser-extension/src/services/piston.ts` around lines 4 - 6, Replace the template-literal construction of the Piston baseUrl with the explicit new URL pattern used elsewhere: instead of stringifying env.VITE_PUBLIC_WEBSITE_URL inside the template literal for the Piston constructor, build the base URL via new URL('api/piston', env.VITE_PUBLIC_WEBSITE_URL) so it matches the codebase convention; apply the same change to the PostHog initialization (posthog.ts) which currently uses the identical template-literal approach.packages/piston.ts/src/fetch.ts (2)
18-30: Off-by-one clarity: this allows 4 total requests (1 initial + 3 retries).The check
attempts >= 3fires before incrementing, so the loop permitsattemptsto reach 3 before throwing. This means the function makes up to 4 HTTP requests in total (the initial request plus 3 retries). If the intent is exactly 3 total attempts, the condition should beattempts >= 2. Worth confirming the desired behaviour.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/piston.ts/src/fetch.ts` around lines 18 - 30, The rate-limit retry logic in the response.status === 429 block lets the code make 4 requests (initial + 3 retries) because the attempts check happens before incrementing; fix by either checking after incrementing or lowering the threshold to attempts >= 2 so total attempts become 3, e.g., update the conditional that throws the Error (currently "if (attempts >= 3)") to reflect the intended max attempts (or move attempts++ above the check); adjust the throw (throw new Error('Too many requests', { cause: response })) to run when the intended retry limit is reached.
1-33: Consider adding anAbortSignal/ timeout to prevent indefinite hangs.If the upstream Piston API becomes unresponsive,
fetchwill block indefinitely (particularly in Node.js environments). Accepting an optionalAbortSignalor wiring in a default timeout viaAbortSignal.timeout(ms)would improve resilience.♻️ Example
-export async function betterFetch(input: RequestInfo, init?: RequestInit) { +export async function betterFetch(input: RequestInfo, init?: RequestInit & { timeoutMs?: number }) { let attempts = 0; while (true) { - const [error, response] = await fetch(input, init) + const signal = init?.signal ?? (init?.timeoutMs ? AbortSignal.timeout(init.timeoutMs) : undefined); + const [error, response] = await fetch(input, { ...init, signal })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/piston.ts/src/fetch.ts` around lines 1 - 33, The betterFetch function can hang if the upstream is unresponsive—update it to accept an optional AbortSignal (e.g., add a third param signal?: AbortSignal) and/or create an internal timeout signal via AbortController (use AbortSignal.timeout(defaultMs) or create a controller that aborts after X ms) and pass the composed signal into fetch (merge provided signal with internal timeout so user signal cancels too); ensure retries recreate or reuse the timeout signal appropriately, handle AbortError responses by throwing a clear error (preserve cause), and avoid leaking timers/controllers between attempts in the betterFetch implementation.packages/piston.ts/src/client.ts (1)
16-18: Make the constructor options optional to use defaults cleanly.Requiring an options object forces callers to pass
{}even when defaults are enough. Allowing an omitted parameter keeps behaviour unchanged while improving ergonomics.Proposed tweak
- constructor(options: typeof ClientOptions._input) { - this.#options = ClientOptions.parse(options); - } + constructor(options: typeof ClientOptions._input = {}) { + this.#options = ClientOptions.parse(options); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/piston.ts/src/client.ts` around lines 16 - 18, Make the constructor parameter optional so callers can omit options; update the constructor signature for the class that sets this.#options (change the parameter to optional) and call ClientOptions.parse with a safe default (e.g., pass options ?? {} or otherwise ensure parse receives an empty object when undefined) so existing default behavior is preserved while allowing callers to omit the argument; reference the constructor, this.#options, and ClientOptions.parse/ClientOptions._input when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/discord-bot/src/env.ts`:
- Line 8: The env schema made PISTON_API_KEY mandatory (PISTON_API_KEY:
z.string().min(1)) which will break deployments missing the key; either change
the schema to optional (PISTON_API_KEY: z.string().min(1).optional()) and add a
runtime guard where the key is consumed (the code that initializes or calls the
Piston client / PISTON_API_KEY consumers) to skip or error gracefully when
absent, or keep it required but update deployment docs and CI/secret stores to
ensure the variable is present; update the code paths that assume the key
accordingly (look for PISTON_API_KEY usage/initialization sites) so behavior is
consistent with your chosen approach.
In `@apps/website/src/app/api/piston/`[route]/route.ts:
- Around line 12-55: The endpoint lacks CORS response headers and currently
forwards OPTIONS upstream, so browsers block allowed origins; update
withOriginCheck to short‑circuit preflight requests (if req.method ===
'OPTIONS') returning a 204/empty Response with Access-Control-Allow-Origin set
to the incoming origin plus required headers like Access-Control-Allow-Methods
(e.g. "GET, POST, OPTIONS") and Access-Control-Allow-Headers (e.g.
"Content-Type, Authorization") and Access-Control-Max-Age; also ensure
handleRequest (and the wrapper that returns upstream responses) attaches
Access-Control-Allow-Origin: <origin> (and optionally Vary: Origin) to proxied
responses so GET/POST responses include the header; adjust exports
(GET/POST/OPTIONS) to use the updated withOriginCheck/handlers accordingly.
In `@apps/website/src/env.ts`:
- Around line 12-15: The ALLOWED_ORIGIN schema currently defaults to '*' which
is a security risk; update the ALLOWED_ORIGIN definition in env.ts to avoid the
wildcard default by either removing .default('*') and making the variable
required (e.g., mirror the PISTON_API_KEY pattern) or change the default to an
empty string ('.default('')') so no origins are allowed when unset, while
keeping the .transform((v) => v.split(',').map((o) => o.trim())) logic intact;
ensure validation reflects the chosen approach (required/non-empty or empty-list
handling).
In `@package.json`:
- Line 4: The package.json "workspaces" array includes non-existent globs
'bots/*' and 'extensions/*'; remove those two entries from the "workspaces"
array so it only lists actual workspace globs (e.g., keep "apps/*" and
"packages/*"), ensuring the "workspaces" array is a valid JSON array without
trailing commas and only contains existing workspace patterns.
- Around line 27-29: Move the patchedDependencies entry under the existing
"pnpm" object and fix the package key to use a single version string: change the
key from "@tailwindcss/vite@4.1.11@4.1.11" to "@tailwindcss/vite@4.1.11" inside
the "patchedDependencies" map (ensure the symbol patchedDependencies is nested
as pnpm.patchedDependencies). Also update the "workspaces" array to remove the
dead globs "bots/*" and "extensions/*" (or create those directories) so
workspace entries reflect actual repo folders.
In `@packages/piston.ts/src/execute.ts`:
- Around line 40-46: The current transform that computes length and lines from
Object.values(o.files) counts metadata keys like "::args::" and "::input::";
update the transform in execute.ts (the .transform((o) => { ... })) to filter
o.files entries by key before reducing—exclude keys "::args::" and "::input::"
(and any other non-code keys) so that the reduce calculating length and lines
only sums actual code file contents, then return the same { ...o, lines, length,
focused: o.focused ?? o.entry } using the filtered values.
---
Nitpick comments:
In @.env/.env.example:
- Around line 4-6: The "Piston (Required)" header incorrectly implies
ALLOWED_ORIGIN is required; update the .env example to mark PISTON_API_KEY as
required but annotate ALLOWED_ORIGIN as optional (or show a separate "Optional"
line) and mention its server-side default '*' (see env.ts
z.string().default('*')) so contributors know it falls back to allowing all
origins when unset; keep variable names PISTON_API_KEY and ALLOWED_ORIGIN
unchanged.
In `@apps/browser-extension/src/services/piston.ts`:
- Around line 4-6: Replace the template-literal construction of the Piston
baseUrl with the explicit new URL pattern used elsewhere: instead of
stringifying env.VITE_PUBLIC_WEBSITE_URL inside the template literal for the
Piston constructor, build the base URL via new URL('api/piston',
env.VITE_PUBLIC_WEBSITE_URL) so it matches the codebase convention; apply the
same change to the PostHog initialization (posthog.ts) which currently uses the
identical template-literal approach.
In `@apps/discord-bot/src/commands/evaluate.ts`:
- Around line 50-63: The autocomplete method duplicates the same slice-and-map
response logic in both branches; refactor autocomplete to compute runtimes via
either piston.runtimes.search(runtime) when runtime is present or
piston.runtimes() otherwise, then apply the shared transformation
runtimes.slice(0, 25).map(r => ({ name: r.name, value: r.id })) and call
interaction.respond once; update the function (autocomplete) so only the
runtime-fetch differs and the mapping/responding is centralized, referencing
piston.runtimes.search, piston.runtimes, and interaction.respond.
In `@packages/piston.ts/src/arguments.ts`:
- Line 1: Replace the informal file header comment "// `@ts-expect-error` - Lexure
has crap typings" with a concise, professional `@ts-expect-error` note that
explains which Lexure typings are incorrect or missing, what symbols/types in
this file are affected (so future contributors can understand the reason), and
optionally reference a tracking GitHub issue or PR number and a TODO to remove
the suppression once the upstream types are fixed; locate the existing string to
update and ensure the new comment remains immediately above the offending
import/line so the `@ts-expect-error` still applies.
In `@packages/piston.ts/src/client.ts`:
- Around line 16-18: Make the constructor parameter optional so callers can omit
options; update the constructor signature for the class that sets this.#options
(change the parameter to optional) and call ClientOptions.parse with a safe
default (e.g., pass options ?? {} or otherwise ensure parse receives an empty
object when undefined) so existing default behavior is preserved while allowing
callers to omit the argument; reference the constructor, this.#options, and
ClientOptions.parse/ClientOptions._input when making the change.
In `@packages/piston.ts/src/fetch.ts`:
- Around line 18-30: The rate-limit retry logic in the response.status === 429
block lets the code make 4 requests (initial + 3 retries) because the attempts
check happens before incrementing; fix by either checking after incrementing or
lowering the threshold to attempts >= 2 so total attempts become 3, e.g., update
the conditional that throws the Error (currently "if (attempts >= 3)") to
reflect the intended max attempts (or move attempts++ above the check); adjust
the throw (throw new Error('Too many requests', { cause: response })) to run
when the intended retry limit is reached.
- Around line 1-33: The betterFetch function can hang if the upstream is
unresponsive—update it to accept an optional AbortSignal (e.g., add a third
param signal?: AbortSignal) and/or create an internal timeout signal via
AbortController (use AbortSignal.timeout(defaultMs) or create a controller that
aborts after X ms) and pass the composed signal into fetch (merge provided
signal with internal timeout so user signal cancels too); ensure retries
recreate or reuse the timeout signal appropriately, handle AbortError responses
by throwing a clear error (preserve cause), and avoid leaking timers/controllers
between attempts in the betterFetch implementation.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (62)
.env/.env.development.env/.env.exampleapps/browser-extension/package.jsonapps/browser-extension/src/background/index.tsapps/browser-extension/src/content-script/execution/dialog.tsxapps/browser-extension/src/content-script/execution/index.tsxapps/browser-extension/src/content-script/execution/result.tsxapps/browser-extension/src/services/piston.tsapps/discord-bot/package.jsonapps/discord-bot/src/commands/evaluate.tsapps/discord-bot/src/env.tsapps/discord-bot/src/handlers/evaluate.tsapps/discord-bot/src/locales/en/messages.poapps/discord-bot/src/services/piston.tsapps/website/package.jsonapps/website/src/app/[locale]/(editor)/playgrounds/[playground]/page.tsxapps/website/src/app/[locale]/(playgrounds)/playgrounds/page.tsxapps/website/src/app/[locale]/(playgrounds)/playgrounds/playground-card-list.tsxapps/website/src/app/[locale]/(playgrounds)/playgrounds/playground-card.tsxapps/website/src/app/api/piston/[route]/route.tsapps/website/src/components/editor/execute-bar/index.tsxapps/website/src/components/editor/index.tsxapps/website/src/components/explorer/use.tsxapps/website/src/components/terminal/index.tsxapps/website/src/components/terminal/use.tsxapps/website/src/env.tsapps/website/src/locales/en/messages.poapps/website/src/services/piston.tspackage.jsonpackages/execute/package.jsonpackages/execute/src/fetch.tspackages/execute/src/shapes.tspackages/execute/src/url.tspackages/fetch/package.jsonpackages/fetch/src/http-error.tspackages/fetch/tsconfig.jsonpackages/piston.ts/package.jsonpackages/piston.ts/readme.mdpackages/piston.ts/src/arguments.tspackages/piston.ts/src/callable.tspackages/piston.ts/src/client.tspackages/piston.ts/src/data/examples.jsonpackages/piston.ts/src/data/extensions.jsonpackages/piston.ts/src/data/icons.jsonpackages/piston.ts/src/data/names.jsonpackages/piston.ts/src/evaluate/compress.tspackages/piston.ts/src/evaluate/index.tspackages/piston.ts/src/evaluate/url.tspackages/piston.ts/src/execute.tspackages/piston.ts/src/fetch.tspackages/piston.ts/src/getters.tspackages/piston.ts/src/index.tspackages/piston.ts/src/runtimes.tspackages/piston.ts/tsconfig.jsonpackages/runtimes/package.jsonpackages/runtimes/src/fetch.tspackages/runtimes/src/index.tspackages/runtimes/src/shapes.tspackages/runtimes/tsconfig.jsonpackages/scripts/bin/ff-zippackages/scripts/package.jsonturbo.json
💤 Files with no reviewable changes (14)
- packages/runtimes/src/index.ts
- packages/runtimes/src/fetch.ts
- packages/runtimes/tsconfig.json
- packages/execute/src/fetch.ts
- packages/execute/src/url.ts
- packages/scripts/package.json
- packages/fetch/tsconfig.json
- packages/fetch/src/http-error.ts
- packages/execute/src/shapes.ts
- packages/piston.ts/src/evaluate/index.ts
- packages/runtimes/src/shapes.ts
- packages/runtimes/package.json
- packages/execute/package.json
- packages/fetch/package.json
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Combine execute, runtimes, and fetch into piston.ts package, support for passing api key to client, setup proxy in order to attach api key to piston requests
Summary by CodeRabbit
New Features
Bug Fixes