feat(sdk): add @superset_sh/sdk — TypeScript SDK mirroring the CLI 1:1#3937
Merged
saddlepaddle merged 14 commits intomainfrom May 1, 2026
Merged
feat(sdk): add @superset_sh/sdk — TypeScript SDK mirroring the CLI 1:1#3937saddlepaddle merged 14 commits intomainfrom
saddlepaddle merged 14 commits intomainfrom
Conversation
External-client-facing SDK that wraps the same procedures the CLI uses, so consumers get a typed, npm-installable surface for tasks, workspaces, projects, hosts, and automations without writing tRPC plumbing. - API path: tasks/projects/hosts/workspaces.list/automations CRUD via /api/trpc with x-api-key + x-superset-organization-id headers - Relay path: workspaces.create/delete and automations.run reach the host service via the relay tunnel; SDK lazily exchanges the API key for a short-lived JWT (the relay only verifies JWTs) - Hand-rolled types (strings for dates) so the package is self-contained with zero runtime deps — no @superset/trpc import, makes the eventual repo split a copy-paste - Stainless skeleton (client / APIResource / APIPromise / errors) lifted from stainless-sdks/superset-typescript under Apache-2.0; biome is skipped on packages/sdk/src so vendor formatting stays intact Verified end-to-end against prod: read paths for every resource, workspace create→delete round-trip on a tunneled host, and automation pause/resume/run dispatch through the relay. Published as @superset_sh/sdk@0.0.1-alpha.3.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new TypeScript SDK package Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Client as Superset Client
participant Builder as Request Builder
participant Fetch as Fetch Layer
participant Parser as Response Parser
participant Retry as Retry Logic
App->>Client: client.tasks.list(query)
Client->>Builder: buildRequest(options)
Builder->>Builder: buildURL, headers, body
Builder->>Fetch: fetch(url, init)
Fetch-->>Parser: response
Parser->>Client: parsed data
alt transient/network error or 5xx/429/408/409
Fetch-->>Retry: error/response
Retry->>Fetch: retry with backoff
end
Client-->>App: returns APIPromise<TaskListResponse>
sequenceDiagram
participant App as Application
participant Client as Superset Client
participant Auth as Auth Cache
participant Relay as Auth Endpoint
participant Host as Host Service
App->>Client: client.workspaces.create(params)
Client->>Auth: getJWT(organizationId)
alt cached
Auth-->>Client: jwt
else
Auth->>Relay: POST /api/auth/token (apiKey)
Relay-->>Auth: jwt
Auth->>Auth: cache(jwt)
Auth-->>Client: jwt
end
Client->>Host: hostMutation(workspace.create, jwt)
Host-->>Client: HostWorkspace
Client-->>App: HostWorkspace
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Greptile SummaryIntroduces Two issues need attention before merge:
Confidence Score: 3/5Not safe to merge as-is — one security issue (API key logged) and one silent auth failure on relay calls need fixes first. Two P1 findings: API key exposure in debug logs and JWT relay auth silently overwritten by caller options. Both are in the critical paths (logging and relay) and need to be resolved before this ships externally.
|
| Filename | Overview |
|---|---|
| packages/sdk/src/client.ts | Core SDK client — two issues: options spread in hostMutation/hostQuery silently overwrites JWT relay auth headers (P1), and _getJwt lacks an in-flight guard (P2). |
| packages/sdk/src/internal/utils/log.ts | Debug log redaction misses x-api-key — the actual header name used for API key auth — exposing raw keys in debug-level logs (P1 security). |
| packages/sdk/src/resources/automations.ts | Full automation CRUD + run/pause/resume/logs/getPrompt/setPrompt; correctly routes through cloud API for dispatch and maps pause/resume to setEnabled. |
| packages/sdk/src/resources/workspaces.ts | Workspace lifecycle via relay (hostMutation); delete correctly looks up the host from cloud index before routing. Clean orgId guard. |
| packages/sdk/src/resources/tasks.ts | Task CRUD with correct wire-shape unwrapping ({ task, txid } → Task); list denormalization is clean. |
| packages/sdk/scripts/build.ts | Build script generates ESM + CJS bundles and .d.ts hierarchy, rewrites package.json for publish under @superset_sh/sdk. No issues. |
| packages/sdk/package.json | private: true + publishConfig.name pattern correctly prevents accidental workspace publish while the build script writes a separate dist/package.json without private. |
Sequence Diagram
sequenceDiagram
participant SDK as SDK Client
participant Cloud as api.superset.sh
participant Relay as relay.superset.sh
participant Host as Host Service
Note over SDK,Cloud: Standard tRPC path (tasks/projects/hosts/automations/workspaces.list)
SDK->>Cloud: GET /api/trpc/<proc>?input=… (x-api-key or Bearer JWT)
Cloud-->>SDK: { result: { data: { json: … } } }
Note over SDK,Host: Relay path (workspaces.create / workspaces.delete)
SDK->>Cloud: GET /api/auth/token (x-api-key) [cached 55 min]
Cloud-->>SDK: { token: "<1h JWT>" }
SDK->>Relay: POST /hosts/<orgId>:<hostId>/trpc/<proc> (Authorization: Bearer jwt)
Relay->>Host: Forward tRPC mutation
Host-->>Relay: Result
Relay-->>SDK: { result: { data: { json: … } } }
Note over SDK,Cloud: automations.run (cloud dispatches to host)
SDK->>Cloud: POST /api/trpc/automation.runNow (x-api-key)
Cloud->>Host: Relay dispatch (server-side)
Cloud-->>SDK: AutomationRun
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
packages/sdk/src/internal/utils/log.ts:113-119
**`x-api-key` header not redacted in debug logs**
The redaction list checks for the header name `api_key` (underscore), but the SDK's `authHeaders` method sends the API key under `x-api-key` (dash). Any caller with `logLevel: 'debug'` will have their raw `sk_live_…` key written to logs.
```suggestion
name.toLowerCase() === "api_key" ||
name.toLowerCase() === "x-api-key" ||
name.toLowerCase() === "authorization" ||
```
### Issue 2 of 3
packages/sdk/src/client.ts:493-501
**`options` spread overwrites relay JWT auth headers**
The relay-specific JWT headers are assembled first, but `...options` is spread after them. If the caller passes any `options.headers` (e.g. `{ headers: { 'X-Trace-Id': '…' } }`), JavaScript's object spread replaces the entire `headers` key, silently dropping both `Authorization: Bearer <jwt>` and `"x-api-key": null`. The relay call then hits the endpoint with the default API-key auth that the relay does not accept. The same issue exists in `hostQuery` at the symmetric spread. Fix by moving `...options` before the `headers` field and merging `options.headers` inside the `headers` object.
### Issue 3 of 3
packages/sdk/src/client.ts:545-574
**JWT refresh race condition (thundering herd)**
`_getJwt` is `async` but has no in-flight promise guard. If two concurrent `hostMutation` / `hostQuery` calls both read a stale or absent cache before either write resolves, each will issue its own `GET /api/auth/token` request. Store the pending promise so concurrent callers await the same fetch.
Reviews (1): Last reviewed commit: "feat(sdk): add @superset_sh/sdk — TypeSc..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+113
to
+119
| name, | ||
| name.toLowerCase() === "api_key" || | ||
| name.toLowerCase() === "authorization" || | ||
| name.toLowerCase() === "cookie" || | ||
| name.toLowerCase() === "set-cookie" | ||
| ? "***" | ||
| : value, |
There was a problem hiding this comment.
x-api-key header not redacted in debug logs
The redaction list checks for the header name api_key (underscore), but the SDK's authHeaders method sends the API key under x-api-key (dash). Any caller with logLevel: 'debug' will have their raw sk_live_… key written to logs.
Suggested change
| name, | |
| name.toLowerCase() === "api_key" || | |
| name.toLowerCase() === "authorization" || | |
| name.toLowerCase() === "cookie" || | |
| name.toLowerCase() === "set-cookie" | |
| ? "***" | |
| : value, | |
| name.toLowerCase() === "api_key" || | |
| name.toLowerCase() === "x-api-key" || | |
| name.toLowerCase() === "authorization" || |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/sdk/src/internal/utils/log.ts
Line: 113-119
Comment:
**`x-api-key` header not redacted in debug logs**
The redaction list checks for the header name `api_key` (underscore), but the SDK's `authHeaders` method sends the API key under `x-api-key` (dash). Any caller with `logLevel: 'debug'` will have their raw `sk_live_…` key written to logs.
```suggestion
name.toLowerCase() === "api_key" ||
name.toLowerCase() === "x-api-key" ||
name.toLowerCase() === "authorization" ||
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+493
to
+501
| const optsPromise = this._getJwt().then((jwt) => ({ | ||
| body: { json: input ?? null }, | ||
| headers: { | ||
| // Drop the API-key auth — relay only verifies JWTs. | ||
| "x-api-key": null, | ||
| Authorization: `Bearer ${jwt}`, | ||
| } as HeadersLike, | ||
| ...options, | ||
| })); |
There was a problem hiding this comment.
options spread overwrites relay JWT auth headers
The relay-specific JWT headers are assembled first, but ...options is spread after them. If the caller passes any options.headers (e.g. { headers: { 'X-Trace-Id': '…' } }), JavaScript's object spread replaces the entire headers key, silently dropping both Authorization: Bearer <jwt> and "x-api-key": null. The relay call then hits the endpoint with the default API-key auth that the relay does not accept. The same issue exists in hostQuery at the symmetric spread. Fix by moving ...options before the headers field and merging options.headers inside the headers object.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/sdk/src/client.ts
Line: 493-501
Comment:
**`options` spread overwrites relay JWT auth headers**
The relay-specific JWT headers are assembled first, but `...options` is spread after them. If the caller passes any `options.headers` (e.g. `{ headers: { 'X-Trace-Id': '…' } }`), JavaScript's object spread replaces the entire `headers` key, silently dropping both `Authorization: Bearer <jwt>` and `"x-api-key": null`. The relay call then hits the endpoint with the default API-key auth that the relay does not accept. The same issue exists in `hostQuery` at the symmetric spread. Fix by moving `...options` before the `headers` field and merging `options.headers` inside the `headers` object.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+545
to
+574
| private async _getJwt(): Promise<string> { | ||
| const now = Date.now(); | ||
| if (this._jwtCache && this._jwtCache.expiresAt - 5 * 60_000 > now) { | ||
| return this._jwtCache.token; | ||
| } | ||
| const headers: Record<string, string> = | ||
| this.apiKey.startsWith("sk_live_") || this.apiKey.startsWith("sk_test_") | ||
| ? { "x-api-key": this.apiKey } | ||
| : { Authorization: `Bearer ${this.apiKey}` }; | ||
| const res = await this.fetch.call( | ||
| undefined, | ||
| `${this.baseURL}/api/auth/token`, | ||
| { | ||
| method: "GET", | ||
| headers, | ||
| }, | ||
| ); | ||
| if (!res.ok) { | ||
| throw new Errors.SupersetError( | ||
| `Failed to exchange API key for JWT (HTTP ${res.status}). The API key may be invalid or revoked.`, | ||
| ); | ||
| } | ||
| const body = (await res.json()) as { token?: string }; | ||
| if (!body.token) { | ||
| throw new Errors.SupersetError("Auth token endpoint returned no token"); | ||
| } | ||
| // Server issues 1h JWTs; cache for 55 minutes to be safe. | ||
| this._jwtCache = { token: body.token, expiresAt: now + 55 * 60_000 }; | ||
| return body.token; | ||
| } |
There was a problem hiding this comment.
_getJwt is async but has no in-flight promise guard. If two concurrent hostMutation / hostQuery calls both read a stale or absent cache before either write resolves, each will issue its own GET /api/auth/token request. Store the pending promise so concurrent callers await the same fetch.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/sdk/src/client.ts
Line: 545-574
Comment:
**JWT refresh race condition (thundering herd)**
`_getJwt` is `async` but has no in-flight promise guard. If two concurrent `hostMutation` / `hostQuery` calls both read a stale or absent cache before either write resolves, each will issue its own `GET /api/auth/token` request. Store the pending promise so concurrent callers await the same fetch.
How can I resolve this? If you propose a fix, please make it concise.![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: 12
🧹 Nitpick comments (5)
packages/sdk/src/internal/errors.ts (1)
3-33: ⚡ Quick winTighten error helpers by removing
anyand using explicit narrowing.These helpers currently bypass type checks with
any. Please switch tounknown+ guarded access to keep this path type-safe.Proposed refactor
+type ErrorLike = { + name?: unknown; + message?: unknown; + stack?: unknown; + cause?: unknown; +}; + +const isErrorLike = (value: unknown): value is ErrorLike => + typeof value === 'object' && value !== null; + export function isAbortError(err: unknown) { return ( - typeof err === 'object' && - err !== null && + isErrorLike(err) && // Spec-compliant fetch implementations - (('name' in err && (err as any).name === 'AbortError') || + (('name' in err && err.name === 'AbortError') || // Expo fetch - ('message' in err && String((err as any).message).includes('FetchRequestCanceledException'))) + ('message' in err && String(err.message).includes('FetchRequestCanceledException'))) ); } -export const castToError = (err: any): Error => { +export const castToError = (err: unknown): Error => { if (err instanceof Error) return err; - if (typeof err === 'object' && err !== null) { + if (isErrorLike(err)) { try { if (Object.prototype.toString.call(err) === '[object Error]') { - // `@ts-ignore` - not all envs have native support for cause yet - const error = new Error(err.message, err.cause ? { cause: err.cause } : {}); + const message = typeof err.message === 'string' ? err.message : String(err.message); + // `@ts-ignore` - not all envs have native support for cause yet + const error = new Error(message, err.cause ? { cause: err.cause } : {}); if (err.stack) error.stack = err.stack; // `@ts-ignore` - not all envs have native support for cause yet if (err.cause && !error.cause) error.cause = err.cause; if (err.name) error.name = err.name; return error; } } catch {} try { return new Error(JSON.stringify(err)); } catch {} } - return new Error(err); + return new Error(typeof err === 'string' ? err : String(err)); };As per coding guidelines,
**/*.{ts,tsx}: Avoid usinganytype; prefer explicit type safety in TypeScript.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/internal/errors.ts` around lines 3 - 33, Change the loose `any` usage in the error helpers to `unknown` and add explicit narrowing/guards: update the signature of castToError to accept `err: unknown`, adjust the internal checks in castToError to use typeof checks, `err !== null`, and `in` operator before accessing `message`, `stack`, `cause`, and `name` (instead of blind casts), and only attempt JSON.stringify when `err` is an object; ensure the final fallback constructs Error with String(err) to avoid passing non-strings into Error; similarly ensure isAbortError continues using safe guards (`typeof err === 'object' && err !== null` and `in` checks) rather than casts.packages/sdk/src/internal/shim-types.ts (1)
10-24: ⚡ Quick win
anyin stream shim types weakens the exported type surface.Please replace
anydefaults/fallbacks withunknown(or a stricter generic bound) so consumers keep type safety.Proposed diff
-type NeverToAny<T> = T extends never ? any : T; +type NeverToUnknown<T> = T extends never ? unknown : T; /** `@ts-ignore` */ -type _DOMReadableStream<R = any> = globalThis.ReadableStream<R>; +type _DOMReadableStream<R = unknown> = globalThis.ReadableStream<R>; /** `@ts-ignore` */ -type _NodeReadableStream<R = any> = import('stream/web').ReadableStream<R>; +type _NodeReadableStream<R = unknown> = import("stream/web").ReadableStream<R>; -type _ConditionalNodeReadableStream<R = any> = - typeof globalThis extends { ReadableStream: any } ? never : _NodeReadableStream<R>; +type _ConditionalNodeReadableStream<R = unknown> = + typeof globalThis extends { ReadableStream: unknown } ? never : _NodeReadableStream<R>; -type _ReadableStream<R = any> = NeverToAny< +type _ReadableStream<R = unknown> = NeverToUnknown< | ([0] extends [1 & _DOMReadableStream<R>] ? never : _DOMReadableStream<R>) | ([0] extends [1 & _ConditionalNodeReadableStream<R>] ? never : _ConditionalNodeReadableStream<R>) >;As per coding guidelines,
**/*.{ts,tsx}: Avoid usinganytype; prefer explicit type safety in TypeScript.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/internal/shim-types.ts` around lines 10 - 24, The exported shim types use `any` as generic defaults and in the NeverToAny alias which weakens consumer type safety; update NeverToAny to use unknown (e.g., type NeverToAny<T> = T extends never ? unknown : T) and change all generic defaults and fallbacks from `R = any` to `R = unknown` for the symbols _DOMReadableStream, _NodeReadableStream, _ConditionalNodeReadableStream and _ReadableStream so the public API surfaces `unknown` instead of `any`.packages/sdk/src/internal/utils/env.ts (1)
11-15: ⚡ Quick winReplace
globalThis as anywith typed runtime guards.Current casts discard type safety in a core utility; this can stay runtime-equivalent with explicit structural types.
Proposed diff
export const readEnv = (env: string): string | undefined => { - if (typeof (globalThis as any).process !== "undefined") { - return (globalThis as any).process.env?.[env]?.trim() || undefined; + const g = globalThis as typeof globalThis & { + process?: { env?: Record<string, string | undefined> }; + Deno?: { env?: { get?: (key: string) => string | undefined } }; + }; + if (typeof g.process !== "undefined") { + return g.process.env?.[env]?.trim() || undefined; } - if (typeof (globalThis as any).Deno !== "undefined") { - return (globalThis as any).Deno.env?.get?.(env)?.trim() || undefined; + if (typeof g.Deno !== "undefined") { + return g.Deno.env?.get?.(env)?.trim() || undefined; } return undefined; };As per coding guidelines,
**/*.{ts,tsx}: Avoid usinganytype; prefer explicit type safety in TypeScript.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/internal/utils/env.ts` around lines 11 - 15, Replace the unchecked casts "(globalThis as any).process" and "(globalThis as any).Deno" with small, explicit structural runtime guards: declare lightweight local typed views (e.g. const maybeNode = globalThis as unknown as { process?: { env?: Record<string,string> } } and const maybeDeno = globalThis as unknown as { Deno?: { env?: { get?: (k: string) => string | undefined } } }) and then check maybeNode.process and maybeDeno.Deno instead of using any; update the branches that call .process.env?.[env]?.trim() and .Deno.env?.get?.(env)?.trim() to use these typed views so you remove the any casts while preserving the same runtime behavior in the env lookup functions in this file.packages/sdk/src/core/api-promise.ts (1)
23-27: ⚡ Quick winRemove the
anyescape from the no-op executor.
resolve(null as any)is only acting as a superclass placeholder, but it still weakens the type boundary in a core SDK primitive. A typed sentinel likeundefined as neverkeeps the lazy-parse behavior without dropping type safety.Proposed fix
) { super((resolve) => { // this is maybe a bit weird but this has to be a no-op to not implicitly // parse the response body; instead .then, .catch, .finally are overridden // to parse the response - resolve(null as any); + resolve(undefined as never); }); this.#client = client; }As per coding guidelines, Avoid using
anytype; prefer explicit type safety in TypeScript.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/core/api-promise.ts` around lines 23 - 27, Replace the unsafe any escape in the Promise constructor no-op: inside the super((resolve) => { ... }) executor (in api-promise.ts) change the placeholder resolve(null as any) to a typed sentinel that preserves type-safety (for example resolve(undefined as never) or resolve(void 0 as never)), so the subclass still performs a no-op while avoiding use of `any` and keeping the Promise generic boundaries intact.packages/sdk/src/internal/utils/path.ts (1)
35-45: ⚡ Quick winReplace the
anycast in path-parameter validation.Line 42 only needs a
hasOwnProperty-shaped lookup. Keeping this branch typed matters because it processes arbitrary path params before building the final URL.Proposed fix
if ( index !== params.length && (value == null || (typeof value === "object" && // handle values from other realms value.toString === Object.getPrototypeOf( - Object.getPrototypeOf((value as any).hasOwnProperty ?? EMPTY) ?? + Object.getPrototypeOf( + (value as { hasOwnProperty?: unknown }).hasOwnProperty ?? EMPTY, + ) ?? EMPTY, )?.toString)) ) {As per coding guidelines, Avoid using
anytype; prefer explicit type safety in TypeScript.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/internal/utils/path.ts` around lines 35 - 45, The code uses an any cast on (value as any).hasOwnProperty; replace that with a safe, typed lookup so we don't use any — for example use (value as Record<string, unknown>).hasOwnProperty with a fallback to Object.prototype.hasOwnProperty (or directly use Object.prototype.hasOwnProperty.call where needed) in the expression inside the path parameter validation (the branch that checks value and its prototype toString). Update the snippet that currently uses (value as any).hasOwnProperty ?? EMPTY to use a typed Record<string, unknown> cast plus a fallback to Object.prototype.hasOwnProperty (or use Object.prototype.hasOwnProperty.call) so the check remains equivalent but without any.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/sdk/README.md`:
- Around line 3-4: Update the example import path in the README from
'@superset/sdk' to the actual published package name '@superset_sh/sdk'
everywhere it appears (including the code block shown and the occurrences around
lines 42-43); ensure all code samples that import Superset use import Superset
from '@superset_sh/sdk' so copy-pasted examples work for consumers testing the
package.
In `@packages/sdk/src/client.ts`:
- Around line 545-573: The _getJwt implementation currently calls
this.fetch.call directly (skipping the client's timeout/retry/logging/error
mapping), so change it to use the client's normal request pipeline — e.g. call
this.fetchWithTimeout(...) or the existing request wrapper (e.g. this.request or
equivalent) instead of this.fetch.call — so the auth token exchange honors
configured timeout/fetchOptions/retries and uses the same error mapping and
logging; keep the same URL/method/headers and preserve the JWT caching logic
(this._jwtCache) and error checks after the response.
- Around line 493-500: The bug is that caller-supplied options.headers can
overwrite the relay JWT header because ...options is spread after constructing
headers in the optsPromise blocks; change the merge order so caller options are
merged first and then the relay auth headers are applied last. Concretely, in
the optsPromise creation (the block building optsPromise that calls
this._getJwt()) and the similar block around the other request path, merge
options into the result before constructing headers (or merge headers like {
...options.headers, "x-api-key": null, Authorization: `Bearer ${jwt}` } with the
relay headers taking precedence), ensuring the Authorization/ x-api-key values
set for relay cannot be overridden by options.headers.
In `@packages/sdk/src/internal/detect-platform.ts`:
- Around line 73-80: In the EdgeRuntime branch of detect-platform (the code that
builds headers including 'X-Stainless-Runtime-Version'), guard access to
(globalThis as any).process.version the same way Node/Deno do by using an
optional chaining/fallback (e.g. (globalThis as any).process?.version ??
'unknown') or a similar safe check before dereferencing; update the Edge branch
where EdgeRuntime is detected so 'X-Stainless-Runtime-Version' uses that guarded
expression to avoid throwing when process is missing.
In `@packages/sdk/src/internal/parse.ts`:
- Around line 49-58: The shared parser is logging full response bodies (via
loggerFor and formatRequestDetails in parse.ts) which can leak secrets; stop
emitting the raw body by replacing the body field with a safe representation
(e.g., "[REDACTED]" or a truncated/hashed summary or type+length) before calling
loggerFor(...). Update the call that builds formatRequestDetails to use a
redactedBody variable (or omit body entirely) so requestLogID and other metadata
still log but the sensitive payload is never included.
In `@packages/sdk/src/internal/to-file.ts`:
- Around line 115-119: The ResponseLike branch in toFile (checks via
isResponseLike) returns early and never sets a string MIME type, so
blobs/responses end up with type === ""; fix by deriving the MIME type from the
blob/response before returning: read blob.type (or
response.headers.get('content-type')) and prefer options.type ?? blob.type ??
inferredTypeFromName, and ensure the code that extracts a regex match (the
variable named type in the fallback) does not overwrite the final MIME string;
then call makeFile/getBytes with that string MIME type (symbols to locate:
toFile, isResponseLike, makeFile, getBytes and the local variable type).
- Around line 115-119: The toFile flow assumes ResponseLike.url is always an
absolute URL and uses new URL(value.url) which throws for empty or relative URLs
(e.g. new Response(blob).url === ""), so update the isResponseLike handling in
toFile: after getting blob, only attempt to parse value.url when it is a
non-empty string and an absolute URL (wrap new URL(value.url) in a try/catch or
check for an origin/prefix) and if parsing fails or value.url is falsy/relative,
fall back to the generated name (e.g. existing generated timestamp/UUID logic)
before calling makeFile(getBytes(blob), name, options). Ensure you reference
isResponseLike, value.url, toFile, getBytes and makeFile when locating the
change.
In `@packages/sdk/src/internal/uploads.ts`:
- Around line 149-153: The current code uses Promise.all over Object.entries to
call addFormValue, which allows asynchronous conversions to complete
out-of-order and breaks append ordering for repeated keys (e.g., files arrays);
replace the Promise.all(...) call with a sequential loop (for...of over
Object.entries(body || {})) that awaits addFormValue(form, key, value) for each
entry to preserve input order, and make the same change for the other occurrence
referenced around lines 209-218 so both append paths maintain stable ordering.
In `@packages/sdk/src/internal/utils/bytes.ts`:
- Around line 17-33: The encodeUTF8 and decodeUTF8 helpers use (globalThis as
any).TextEncoder/TextDecoder; remove the "as any" casts and instead access
globalThis.TextEncoder and globalThis.TextDecoder with explicit runtime guards,
e.g., check typeof globalThis.TextEncoder === "function" (and TextDecoder)
before instantiating; if missing, throw a clear, deterministic error message (or
provide a documented fallback) so encodeUTF8, decodeUTF8, encodeUTF8_ and
decodeUTF8_ initialize only when the constructors are present and do not rely on
any-casts or produce opaque runtime errors.
In `@packages/sdk/src/internal/utils/log.ts`:
- Around line 107-121: The header-redaction logic in
packages/sdk/src/internal/utils/log.ts currently only masks "api_key" and thus
misses the SDK's actual header name "x-api-key"; update the mapping in the block
that transforms details.headers (the expression that checks name.toLowerCase()
=== ...) to also compare against "x-api-key" (and keep the other checks:
"authorization", "cookie", "set-cookie") so header names are matched
case-insensitively and "x-api-key" values are replaced with "***"; ensure you
modify the code inside the details.headers = Object.fromEntries(...).map(...)
closure so the Header/headers handling (including the Headers instanceof branch)
remains unchanged.
In `@packages/sdk/src/internal/utils/values.ts`:
- Around line 67-83: The helpers coerceInteger and coerceFloat currently return
parseInt/parseFloat results unchecked (allowing NaN); change both to validate
coercion results and throw SupersetError on failure: in coerceInteger, for
typeof number ensure Number.isFinite(value) (and not NaN) before returning
Math.round(value), and for typeof string call parseInt(value, 10) then if
Number.isNaN(result) throw SupersetError with the same contextual message;
similarly in coerceFloat, for typeof number ensure Number.isFinite(value) before
returning it, and for typeof string call parseFloat(value) then if
Number.isNaN(result) throw SupersetError—include the original value and typeof
in the error message for both functions (functions: coerceInteger, coerceFloat,
SupersetError).
In `@packages/sdk/src/resources/workspaces.ts`:
- Around line 64-81: The delete method currently returns
Promise<WorkspaceDeleteResult> and narrows options to { hostId?: string },
losing APIPromise lazy helpers and full RequestOptions; change workspaces.delete
to return APIPromise<WorkspaceDeleteResult> and accept RequestOptions, then use
this._thenUnwrap(...) to chain the host lookup
(this._client.query("v2Workspace.getFromHost", { organizationId:
this._requireOrgId(), id })) and the host mutation
(this._client.hostMutation(hostId, "workspace.delete", { id })) so the combined
flow preserves APIPromise behavior and supports signal/headers/retries/timeouts,
throwing SupersetError if cloud is null as before.
---
Nitpick comments:
In `@packages/sdk/src/core/api-promise.ts`:
- Around line 23-27: Replace the unsafe any escape in the Promise constructor
no-op: inside the super((resolve) => { ... }) executor (in api-promise.ts)
change the placeholder resolve(null as any) to a typed sentinel that preserves
type-safety (for example resolve(undefined as never) or resolve(void 0 as
never)), so the subclass still performs a no-op while avoiding use of `any` and
keeping the Promise generic boundaries intact.
In `@packages/sdk/src/internal/errors.ts`:
- Around line 3-33: Change the loose `any` usage in the error helpers to
`unknown` and add explicit narrowing/guards: update the signature of castToError
to accept `err: unknown`, adjust the internal checks in castToError to use
typeof checks, `err !== null`, and `in` operator before accessing `message`,
`stack`, `cause`, and `name` (instead of blind casts), and only attempt
JSON.stringify when `err` is an object; ensure the final fallback constructs
Error with String(err) to avoid passing non-strings into Error; similarly ensure
isAbortError continues using safe guards (`typeof err === 'object' && err !==
null` and `in` checks) rather than casts.
In `@packages/sdk/src/internal/shim-types.ts`:
- Around line 10-24: The exported shim types use `any` as generic defaults and
in the NeverToAny alias which weakens consumer type safety; update NeverToAny to
use unknown (e.g., type NeverToAny<T> = T extends never ? unknown : T) and
change all generic defaults and fallbacks from `R = any` to `R = unknown` for
the symbols _DOMReadableStream, _NodeReadableStream,
_ConditionalNodeReadableStream and _ReadableStream so the public API surfaces
`unknown` instead of `any`.
In `@packages/sdk/src/internal/utils/env.ts`:
- Around line 11-15: Replace the unchecked casts "(globalThis as any).process"
and "(globalThis as any).Deno" with small, explicit structural runtime guards:
declare lightweight local typed views (e.g. const maybeNode = globalThis as
unknown as { process?: { env?: Record<string,string> } } and const maybeDeno =
globalThis as unknown as { Deno?: { env?: { get?: (k: string) => string |
undefined } } }) and then check maybeNode.process and maybeDeno.Deno instead of
using any; update the branches that call .process.env?.[env]?.trim() and
.Deno.env?.get?.(env)?.trim() to use these typed views so you remove the any
casts while preserving the same runtime behavior in the env lookup functions in
this file.
In `@packages/sdk/src/internal/utils/path.ts`:
- Around line 35-45: The code uses an any cast on (value as any).hasOwnProperty;
replace that with a safe, typed lookup so we don't use any — for example use
(value as Record<string, unknown>).hasOwnProperty with a fallback to
Object.prototype.hasOwnProperty (or directly use
Object.prototype.hasOwnProperty.call where needed) in the expression inside the
path parameter validation (the branch that checks value and its prototype
toString). Update the snippet that currently uses (value as any).hasOwnProperty
?? EMPTY to use a typed Record<string, unknown> cast plus a fallback to
Object.prototype.hasOwnProperty (or use Object.prototype.hasOwnProperty.call) so
the check remains equivalent but without any.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aa1ca2bb-39e3-4799-9de1-3f2e888c5120
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (55)
biome.jsoncpackages/sdk/LICENSEpackages/sdk/README.mdpackages/sdk/api.mdpackages/sdk/package.jsonpackages/sdk/scripts/build.tspackages/sdk/src/api-promise.tspackages/sdk/src/client.tspackages/sdk/src/core/README.mdpackages/sdk/src/core/api-promise.tspackages/sdk/src/core/error.tspackages/sdk/src/core/resource.tspackages/sdk/src/core/uploads.tspackages/sdk/src/error.tspackages/sdk/src/index.tspackages/sdk/src/internal/README.mdpackages/sdk/src/internal/builtin-types.tspackages/sdk/src/internal/detect-platform.tspackages/sdk/src/internal/errors.tspackages/sdk/src/internal/headers.tspackages/sdk/src/internal/parse.tspackages/sdk/src/internal/qs/LICENSE.mdpackages/sdk/src/internal/qs/README.mdpackages/sdk/src/internal/qs/formats.tspackages/sdk/src/internal/qs/index.tspackages/sdk/src/internal/qs/stringify.tspackages/sdk/src/internal/qs/types.tspackages/sdk/src/internal/qs/utils.tspackages/sdk/src/internal/request-options.tspackages/sdk/src/internal/shim-types.tspackages/sdk/src/internal/shims.tspackages/sdk/src/internal/to-file.tspackages/sdk/src/internal/types.tspackages/sdk/src/internal/uploads.tspackages/sdk/src/internal/utils.tspackages/sdk/src/internal/utils/base64.tspackages/sdk/src/internal/utils/bytes.tspackages/sdk/src/internal/utils/env.tspackages/sdk/src/internal/utils/log.tspackages/sdk/src/internal/utils/path.tspackages/sdk/src/internal/utils/query.tspackages/sdk/src/internal/utils/sleep.tspackages/sdk/src/internal/utils/uuid.tspackages/sdk/src/internal/utils/values.tspackages/sdk/src/resource.tspackages/sdk/src/resources.tspackages/sdk/src/resources/automations.tspackages/sdk/src/resources/hosts.tspackages/sdk/src/resources/index.tspackages/sdk/src/resources/projects.tspackages/sdk/src/resources/tasks.tspackages/sdk/src/resources/workspaces.tspackages/sdk/src/uploads.tspackages/sdk/src/version.tspackages/sdk/tsconfig.json
Comment on lines
+73
to
+80
| if (typeof EdgeRuntime !== 'undefined') { | ||
| return { | ||
| 'X-Stainless-Lang': 'js', | ||
| 'X-Stainless-Package-Version': VERSION, | ||
| 'X-Stainless-OS': 'Unknown', | ||
| 'X-Stainless-Arch': `other:${EdgeRuntime}`, | ||
| 'X-Stainless-Runtime': 'edge', | ||
| 'X-Stainless-Runtime-Version': (globalThis as any).process.version, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does an Edge Runtime environment guarantee a Node-style process.version, or should runtime metadata be derived from the EdgeRuntime global instead?
💡 Result:
No, an Edge Runtime environment does not guarantee a Node-style process.version. The process.version property, which is a Node.js API, is not available or is undefined in Edge Runtime, as it lacks full Node.js APIs. Instead, runtime metadata should be derived from the EdgeRuntime global, accessible as globalThis.EdgeRuntime, which is a string in Edge Runtime (e.g., containing version information like { version: VERSION }) and undefined or not a string in Node.js environments. This allows reliable runtime detection and access to Edge-specific metadata.
Citations:
- 1: Include a key in process.versions for detecting edge runtime vercel/edge-runtime#527
- 2: https://vercel.com/docs/functions/runtimes/edge
- 3:
process.versionis undefined in app dir route vercel/next.js#54739 - 4: https://vercel.com/docs/concepts/functions/edge-functions/edge-runtime
- 5: https://github.com/vercel/edge-runtime/blob/main/docs/pages/features/available-apis.mdx
- 6: https://vercel.com/docs/functions/edge-functions/edge-runtime
- 7: https://github.com/vercel/edge-runtime/blob/0b11a95e2f470d278db27982e4905febc6ac9bb7/packages/runtime/src/edge-runtime.ts
🏁 Script executed:
wc -l packages/sdk/src/internal/detect-platform.tsRepository: superset-sh/superset
Length of output: 113
🏁 Script executed:
sed -n '1,100p' packages/sdk/src/internal/detect-platform.tsRepository: superset-sh/superset
Length of output: 3145
🏁 Script executed:
# Check for any guards or optional chaining in the Edge branch
rg -A 15 "typeof EdgeRuntime" packages/sdk/src/internal/detect-platform.tsRepository: superset-sh/superset
Length of output: 1070
🏁 Script executed:
# Check if process is accessed elsewhere in the file
rg "process\." packages/sdk/src/internal/detect-platform.tsRepository: superset-sh/superset
Length of output: 521
Guard the Edge branch when process is missing.
Line 80 dereferences process.version unconditionally inside the Edge branch. Edge Runtime does not provide Node-style process.version; this will throw an error at runtime. The Node and Deno branches use fallbacks (?? 'unknown'), but the Edge branch does not, making it inconsistent and broken.
Proposed fix
if (typeof EdgeRuntime !== 'undefined') {
+ const edgeGlobal = globalThis as { process?: { version?: string } };
return {
'X-Stainless-Lang': 'js',
'X-Stainless-Package-Package-Version': VERSION,
'X-Stainless-OS': 'Unknown',
'X-Stainless-Arch': `other:${EdgeRuntime}`,
'X-Stainless-Runtime': 'edge',
- 'X-Stainless-Runtime-Version': (globalThis as any).process.version,
+ 'X-Stainless-Runtime-Version':
+ edgeGlobal.process?.version ??
+ (typeof EdgeRuntime === 'string' ? EdgeRuntime : 'unknown'),
};
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sdk/src/internal/detect-platform.ts` around lines 73 - 80, In the
EdgeRuntime branch of detect-platform (the code that builds headers including
'X-Stainless-Runtime-Version'), guard access to (globalThis as
any).process.version the same way Node/Deno do by using an optional
chaining/fallback (e.g. (globalThis as any).process?.version ?? 'unknown') or a
similar safe check before dereferencing; update the Edge branch where
EdgeRuntime is detected so 'X-Stainless-Runtime-Version' uses that guarded
expression to avoid throwing when process is missing.
Comment on lines
+49
to
+58
| loggerFor(client).debug( | ||
| `[${requestLogID}] response parsed`, | ||
| formatRequestDetails({ | ||
| retryOfRequestLogID, | ||
| url: response.url, | ||
| status: response.status, | ||
| body, | ||
| durationMs: Date.now() - startTime, | ||
| }), | ||
| ); |
Contributor
There was a problem hiding this comment.
Don't log full response bodies from the shared parser.
This runs for every request and currently emits the parsed body verbatim. That includes sensitive payloads like the relay JWT from /api/auth/token and automation prompt/log content, so enabling debug logging will leak secrets and user data into application logs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sdk/src/internal/parse.ts` around lines 49 - 58, The shared parser
is logging full response bodies (via loggerFor and formatRequestDetails in
parse.ts) which can leak secrets; stop emitting the raw body by replacing the
body field with a safe representation (e.g., "[REDACTED]" or a truncated/hashed
summary or type+length) before calling loggerFor(...). Update the call that
builds formatRequestDetails to use a redactedBody variable (or omit body
entirely) so requestLogID and other metadata still log but the sensitive payload
is never included.
Comment on lines
+149
to
+153
| await Promise.all( | ||
| Object.entries(body || {}).map(([key, value]) => | ||
| addFormValue(form, key, value), | ||
| ), | ||
| ); |
Contributor
There was a problem hiding this comment.
Preserve FormData append order instead of using Promise.all().
These branches append fields in completion order, not input order. Arrays like files: [streamA, streamB] can therefore serialize as files[]=b&files[]=a if one conversion finishes earlier, which changes the payload semantics for repeated keys.
Suggested fix
const form = new FormData();
- await Promise.all(
- Object.entries(body || {}).map(([key, value]) =>
- addFormValue(form, key, value),
- ),
- );
+ for (const [key, value] of Object.entries(body || {})) {
+ await addFormValue(form, key, value);
+ }
return form;
};
@@
} else if (Array.isArray(value)) {
- await Promise.all(
- value.map((entry) => addFormValue(form, `${key}[]`, entry)),
- );
+ for (const entry of value) {
+ await addFormValue(form, `${key}[]`, entry);
+ }
} else if (typeof value === "object") {
- await Promise.all(
- Object.entries(value).map(([name, prop]) =>
- addFormValue(form, `${key}[${name}]`, prop),
- ),
- );
+ for (const [name, prop] of Object.entries(value)) {
+ await addFormValue(form, `${key}[${name}]`, prop);
+ }
} else {Also applies to: 209-218
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sdk/src/internal/uploads.ts` around lines 149 - 153, The current
code uses Promise.all over Object.entries to call addFormValue, which allows
asynchronous conversions to complete out-of-order and breaks append ordering for
repeated keys (e.g., files arrays); replace the Promise.all(...) call with a
sequential loop (for...of over Object.entries(body || {})) that awaits
addFormValue(form, key, value) for each entry to preserve input order, and make
the same change for the other occurrence referenced around lines 209-218 so both
append paths maintain stable ordering.
Comment on lines
+17
to
+33
| export function encodeUTF8(str: string) { | ||
| let encoder; | ||
| return ( | ||
| encodeUTF8_ ?? | ||
| ((encoder = new (globalThis as any).TextEncoder()), | ||
| (encodeUTF8_ = encoder.encode.bind(encoder))) | ||
| )(str); | ||
| } | ||
|
|
||
| let decodeUTF8_: (bytes: Uint8Array) => string; | ||
| export function decodeUTF8(bytes: Uint8Array) { | ||
| let decoder; | ||
| return ( | ||
| decodeUTF8_ ?? | ||
| ((decoder = new (globalThis as any).TextDecoder()), | ||
| (decodeUTF8_ = decoder.decode.bind(decoder))) | ||
| )(bytes); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists
test -f packages/sdk/src/internal/utils/bytes.ts && echo "File exists" || echo "File not found"Repository: superset-sh/superset
Length of output: 76
🏁 Script executed:
# Read the actual file to verify the code snippet and line numbers
cat -n packages/sdk/src/internal/utils/bytes.ts | head -50Repository: superset-sh/superset
Length of output: 1109
🏁 Script executed:
# Check the specific lines mentioned in the review (17-33)
sed -n '17,33p' packages/sdk/src/internal/utils/bytes.ts | cat -nRepository: superset-sh/superset
Length of output: 630
Remove as any casts and add explicit guards for TextEncoder/TextDecoder constructors.
The code uses (globalThis as any).TextEncoder and (globalThis as any).TextDecoder, which violates the coding guideline to avoid any type in TypeScript. Additionally, accessing these constructors without availability checks can produce opaque runtime errors in unsupported runtimes.
Add explicit typed access to the constructors with runtime checks for deterministic failure modes:
Proposed fix
let encodeUTF8_: (str: string) => Uint8Array;
export function encodeUTF8(str: string) {
let encoder;
+ const TextEncoderCtor = globalThis.TextEncoder;
+ if (!TextEncoderCtor) {
+ throw new Error("TextEncoder is not available in this runtime");
+ }
return (
encodeUTF8_ ??
- ((encoder = new (globalThis as any).TextEncoder()),
+ ((encoder = new TextEncoderCtor()),
(encodeUTF8_ = encoder.encode.bind(encoder)))
)(str);
}
let decodeUTF8_: (bytes: Uint8Array) => string;
export function decodeUTF8(bytes: Uint8Array) {
let decoder;
+ const TextDecoderCtor = globalThis.TextDecoder;
+ if (!TextDecoderCtor) {
+ throw new Error("TextDecoder is not available in this runtime");
+ }
return (
decodeUTF8_ ??
- ((decoder = new (globalThis as any).TextDecoder()),
+ ((decoder = new TextDecoderCtor()),
(decodeUTF8_ = decoder.decode.bind(decoder)))
)(bytes);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function encodeUTF8(str: string) { | |
| let encoder; | |
| return ( | |
| encodeUTF8_ ?? | |
| ((encoder = new (globalThis as any).TextEncoder()), | |
| (encodeUTF8_ = encoder.encode.bind(encoder))) | |
| )(str); | |
| } | |
| let decodeUTF8_: (bytes: Uint8Array) => string; | |
| export function decodeUTF8(bytes: Uint8Array) { | |
| let decoder; | |
| return ( | |
| decodeUTF8_ ?? | |
| ((decoder = new (globalThis as any).TextDecoder()), | |
| (decodeUTF8_ = decoder.decode.bind(decoder))) | |
| )(bytes); | |
| let encodeUTF8_: (str: string) => Uint8Array; | |
| export function encodeUTF8(str: string) { | |
| let encoder; | |
| const TextEncoderCtor = globalThis.TextEncoder; | |
| if (!TextEncoderCtor) { | |
| throw new Error("TextEncoder is not available in this runtime"); | |
| } | |
| return ( | |
| encodeUTF8_ ?? | |
| ((encoder = new TextEncoderCtor()), | |
| (encodeUTF8_ = encoder.encode.bind(encoder))) | |
| )(str); | |
| } | |
| let decodeUTF8_: (bytes: Uint8Array) => string; | |
| export function decodeUTF8(bytes: Uint8Array) { | |
| let decoder; | |
| const TextDecoderCtor = globalThis.TextDecoder; | |
| if (!TextDecoderCtor) { | |
| throw new Error("TextDecoder is not available in this runtime"); | |
| } | |
| return ( | |
| decodeUTF8_ ?? | |
| ((decoder = new TextDecoderCtor()), | |
| (decodeUTF8_ = decoder.decode.bind(decoder))) | |
| )(bytes); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sdk/src/internal/utils/bytes.ts` around lines 17 - 33, The
encodeUTF8 and decodeUTF8 helpers use (globalThis as
any).TextEncoder/TextDecoder; remove the "as any" casts and instead access
globalThis.TextEncoder and globalThis.TextDecoder with explicit runtime guards,
e.g., check typeof globalThis.TextEncoder === "function" (and TextDecoder)
before instantiating; if missing, throw a clear, deterministic error message (or
provide a documented fallback) so encodeUTF8, decodeUTF8, encodeUTF8_ and
decodeUTF8_ initialize only when the constructors are present and do not rely on
any-casts or produce opaque runtime errors.
Comment on lines
+107
to
+121
| if (details.headers) { | ||
| details.headers = Object.fromEntries( | ||
| (details.headers instanceof Headers | ||
| ? [...details.headers] | ||
| : Object.entries(details.headers) | ||
| ).map(([name, value]) => [ | ||
| name, | ||
| name.toLowerCase() === "api_key" || | ||
| name.toLowerCase() === "authorization" || | ||
| name.toLowerCase() === "cookie" || | ||
| name.toLowerCase() === "set-cookie" | ||
| ? "***" | ||
| : value, | ||
| ]), | ||
| ); |
Contributor
There was a problem hiding this comment.
Redact the actual API-key header name.
Line 114 masks api_key, but the SDK authenticates with x-api-key. Request logs will therefore emit live API keys whenever header-based auth is used.
Suggested fix
).map(([name, value]) => [
name,
name.toLowerCase() === "api_key" ||
+ name.toLowerCase() === "x-api-key" ||
name.toLowerCase() === "authorization" ||
name.toLowerCase() === "cookie" ||
name.toLowerCase() === "set-cookie"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (details.headers) { | |
| details.headers = Object.fromEntries( | |
| (details.headers instanceof Headers | |
| ? [...details.headers] | |
| : Object.entries(details.headers) | |
| ).map(([name, value]) => [ | |
| name, | |
| name.toLowerCase() === "api_key" || | |
| name.toLowerCase() === "authorization" || | |
| name.toLowerCase() === "cookie" || | |
| name.toLowerCase() === "set-cookie" | |
| ? "***" | |
| : value, | |
| ]), | |
| ); | |
| if (details.headers) { | |
| details.headers = Object.fromEntries( | |
| (details.headers instanceof Headers | |
| ? [...details.headers] | |
| : Object.entries(details.headers) | |
| ).map(([name, value]) => [ | |
| name, | |
| name.toLowerCase() === "api_key" || | |
| name.toLowerCase() === "x-api-key" || | |
| name.toLowerCase() === "authorization" || | |
| name.toLowerCase() === "cookie" || | |
| name.toLowerCase() === "set-cookie" | |
| ? "***" | |
| : value, | |
| ]), | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sdk/src/internal/utils/log.ts` around lines 107 - 121, The
header-redaction logic in packages/sdk/src/internal/utils/log.ts currently only
masks "api_key" and thus misses the SDK's actual header name "x-api-key"; update
the mapping in the block that transforms details.headers (the expression that
checks name.toLowerCase() === ...) to also compare against "x-api-key" (and keep
the other checks: "authorization", "cookie", "set-cookie") so header names are
matched case-insensitively and "x-api-key" values are replaced with "***";
ensure you modify the code inside the details.headers =
Object.fromEntries(...).map(...) closure so the Header/headers handling
(including the Headers instanceof branch) remains unchanged.
Comment on lines
+67
to
+83
| export const coerceInteger = (value: unknown): number => { | ||
| if (typeof value === "number") return Math.round(value); | ||
| if (typeof value === "string") return parseInt(value, 10); | ||
|
|
||
| throw new SupersetError( | ||
| `Could not coerce ${value} (type: ${typeof value}) into a number`, | ||
| ); | ||
| }; | ||
|
|
||
| export const coerceFloat = (value: unknown): number => { | ||
| if (typeof value === "number") return value; | ||
| if (typeof value === "string") return parseFloat(value); | ||
|
|
||
| throw new SupersetError( | ||
| `Could not coerce ${value} (type: ${typeof value}) into a number`, | ||
| ); | ||
| }; |
Contributor
There was a problem hiding this comment.
Reject invalid numeric coercions instead of returning NaN.
Lines 67-83 return parseInt / parseFloat results unchecked, so inputs like "abc" end up as NaN instead of raising SupersetError. That silently violates the helper's contract and lets bad numeric input leak downstream.
Proposed fix
export const coerceInteger = (value: unknown): number => {
- if (typeof value === "number") return Math.round(value);
- if (typeof value === "string") return parseInt(value, 10);
+ if (typeof value === "number") {
+ const rounded = Math.round(value);
+ if (Number.isFinite(rounded)) return rounded;
+ }
+ if (typeof value === "string") {
+ const parsed = parseInt(value, 10);
+ if (Number.isFinite(parsed)) return parsed;
+ }
throw new SupersetError(
`Could not coerce ${value} (type: ${typeof value}) into a number`,
);
};
export const coerceFloat = (value: unknown): number => {
if (typeof value === "number") return value;
- if (typeof value === "string") return parseFloat(value);
+ if (typeof value === "string") {
+ const parsed = parseFloat(value);
+ if (Number.isFinite(parsed)) return parsed;
+ }
throw new SupersetError(
`Could not coerce ${value} (type: ${typeof value}) into a number`,
);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const coerceInteger = (value: unknown): number => { | |
| if (typeof value === "number") return Math.round(value); | |
| if (typeof value === "string") return parseInt(value, 10); | |
| throw new SupersetError( | |
| `Could not coerce ${value} (type: ${typeof value}) into a number`, | |
| ); | |
| }; | |
| export const coerceFloat = (value: unknown): number => { | |
| if (typeof value === "number") return value; | |
| if (typeof value === "string") return parseFloat(value); | |
| throw new SupersetError( | |
| `Could not coerce ${value} (type: ${typeof value}) into a number`, | |
| ); | |
| }; | |
| export const coerceInteger = (value: unknown): number => { | |
| if (typeof value === "number") { | |
| const rounded = Math.round(value); | |
| if (Number.isFinite(rounded)) return rounded; | |
| } | |
| if (typeof value === "string") { | |
| const parsed = parseInt(value, 10); | |
| if (Number.isFinite(parsed)) return parsed; | |
| } | |
| throw new SupersetError( | |
| `Could not coerce ${value} (type: ${typeof value}) into a number`, | |
| ); | |
| }; | |
| export const coerceFloat = (value: unknown): number => { | |
| if (typeof value === "number") return value; | |
| if (typeof value === "string") { | |
| const parsed = parseFloat(value); | |
| if (Number.isFinite(parsed)) return parsed; | |
| } | |
| throw new SupersetError( | |
| `Could not coerce ${value} (type: ${typeof value}) into a number`, | |
| ); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sdk/src/internal/utils/values.ts` around lines 67 - 83, The helpers
coerceInteger and coerceFloat currently return parseInt/parseFloat results
unchecked (allowing NaN); change both to validate coercion results and throw
SupersetError on failure: in coerceInteger, for typeof number ensure
Number.isFinite(value) (and not NaN) before returning Math.round(value), and for
typeof string call parseInt(value, 10) then if Number.isNaN(result) throw
SupersetError with the same contextual message; similarly in coerceFloat, for
typeof number ensure Number.isFinite(value) before returning it, and for typeof
string call parseFloat(value) then if Number.isNaN(result) throw
SupersetError—include the original value and typeof in the error message for
both functions (functions: coerceInteger, coerceFloat, SupersetError).
Comment on lines
+64
to
+81
| async delete( | ||
| id: string, | ||
| options?: { hostId?: string }, | ||
| ): Promise<WorkspaceDeleteResult> { | ||
| let hostId = options?.hostId; | ||
| if (!hostId) { | ||
| const cloud = await this._client.query<HostLookup | null>( | ||
| "v2Workspace.getFromHost", | ||
| { organizationId: this._requireOrgId(), id }, | ||
| ); | ||
| if (!cloud) throw new SupersetError(`Workspace not found: ${id}`); | ||
| hostId = cloud.hostId; | ||
| } | ||
| return this._client.hostMutation<WorkspaceDeleteResult>( | ||
| hostId, | ||
| "workspace.delete", | ||
| { id }, | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== APIPromise surface ==="
sed -n '1,220p' packages/sdk/src/core/api-promise.ts
echo
echo "=== Client request helpers ==="
rg -n "hostMutation<|query<|mutation<|_thenUnwrap" packages/sdk/src/client.ts -C 2
echo
echo "=== Workspace resource ==="
sed -n '1,140p' packages/sdk/src/resources/workspaces.tsRepository: superset-sh/superset
Length of output: 8122
Align workspaces.delete() with the SDK request surface used by list() and create().
Line 64 currently declares async delete() returning plain Promise<WorkspaceDeleteResult>, so callers lose APIPromise lazy behavior and helpers. The narrowed options type ({ hostId?: string }) also prevents passing RequestOptions like signal, headers, retries, or timeouts. Use _thenUnwrap() to chain the host lookup and delete operations while preserving the APIPromise surface and accepting full RequestOptions:
Implementation sketch
- async delete(
- id: string,
- options?: { hostId?: string },
- ): Promise<WorkspaceDeleteResult> {
- let hostId = options?.hostId;
- if (!hostId) {
- const cloud = await this._client.query<HostLookup | null>(
- "v2Workspace.getFromHost",
- { organizationId: this._requireOrgId(), id },
- );
- if (!cloud) throw new SupersetError(`Workspace not found: ${id}`);
- hostId = cloud.hostId;
- }
- return this._client.hostMutation<WorkspaceDeleteResult>(
- hostId,
- "workspace.delete",
- { id },
- );
- }
+ delete(
+ id: string,
+ options?: RequestOptions & { hostId?: string },
+ ): APIPromise<WorkspaceDeleteResult> {
+ const { hostId, ...requestOptions } = options ?? {};
+ if (hostId) {
+ return this._client.hostMutation<WorkspaceDeleteResult>(
+ hostId,
+ "workspace.delete",
+ { id },
+ requestOptions,
+ );
+ }
+
+ return this._client
+ .query<HostLookup | null>(
+ "v2Workspace.getFromHost",
+ { organizationId: this._requireOrgId(), id },
+ requestOptions,
+ )
+ ._thenUnwrap((cloud) => {
+ if (!cloud) throw new SupersetError(`Workspace not found: ${id}`);
+ return this._client.hostMutation<WorkspaceDeleteResult>(
+ cloud.hostId,
+ "workspace.delete",
+ { id },
+ requestOptions,
+ );
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async delete( | |
| id: string, | |
| options?: { hostId?: string }, | |
| ): Promise<WorkspaceDeleteResult> { | |
| let hostId = options?.hostId; | |
| if (!hostId) { | |
| const cloud = await this._client.query<HostLookup | null>( | |
| "v2Workspace.getFromHost", | |
| { organizationId: this._requireOrgId(), id }, | |
| ); | |
| if (!cloud) throw new SupersetError(`Workspace not found: ${id}`); | |
| hostId = cloud.hostId; | |
| } | |
| return this._client.hostMutation<WorkspaceDeleteResult>( | |
| hostId, | |
| "workspace.delete", | |
| { id }, | |
| ); | |
| delete( | |
| id: string, | |
| options?: RequestOptions & { hostId?: string }, | |
| ): APIPromise<WorkspaceDeleteResult> { | |
| const { hostId, ...requestOptions } = options ?? {}; | |
| if (hostId) { | |
| return this._client.hostMutation<WorkspaceDeleteResult>( | |
| hostId, | |
| "workspace.delete", | |
| { id }, | |
| requestOptions, | |
| ); | |
| } | |
| return this._client | |
| .query<HostLookup | null>( | |
| "v2Workspace.getFromHost", | |
| { organizationId: this._requireOrgId(), id }, | |
| requestOptions, | |
| ) | |
| ._thenUnwrap((cloud) => { | |
| if (!cloud) throw new SupersetError(`Workspace not found: ${id}`); | |
| return this._client.hostMutation<WorkspaceDeleteResult>( | |
| cloud.hostId, | |
| "workspace.delete", | |
| { id }, | |
| requestOptions, | |
| ); | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sdk/src/resources/workspaces.ts` around lines 64 - 81, The delete
method currently returns Promise<WorkspaceDeleteResult> and narrows options to {
hostId?: string }, losing APIPromise lazy helpers and full RequestOptions;
change workspaces.delete to return APIPromise<WorkspaceDeleteResult> and accept
RequestOptions, then use this._thenUnwrap(...) to chain the host lookup
(this._client.query("v2Workspace.getFromHost", { organizationId:
this._requireOrgId(), id })) and the host mutation
(this._client.hostMutation(hostId, "workspace.delete", { id })) so the combined
flow preserves APIPromise behavior and supports signal/headers/retries/timeouts,
throwing SupersetError if cloud is null as before.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
10 issues found across 56 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/sdk/src/internal/utils/log.ts">
<violation number="1" location="packages/sdk/src/internal/utils/log.ts:114">
P1: Sensitive header redaction misses `x-api-key`, so API keys can be leaked in logs.</violation>
</file>
<file name="packages/sdk/README.md">
<violation number="1" location="packages/sdk/README.md:4">
P2: README uses the wrong package name in the main import example (`@superset/sdk` instead of `@superset_sh/sdk`).</violation>
</file>
<file name="packages/sdk/src/internal/to-file.ts">
<violation number="1" location="packages/sdk/src/internal/to-file.ts:127">
P2: The inferred MIME type is never applied because `find(...)` returns the whole part object, not its `type` string.</violation>
</file>
<file name="packages/sdk/src/internal/utils/env.ts">
<violation number="1" location="packages/sdk/src/internal/utils/env.ts:15">
P2: `Deno.env.get` can throw `PermissionDenied` without `allow-env`; this breaks the function contract that inaccessible env vars return `undefined`.</violation>
</file>
<file name="packages/sdk/src/internal/detect-platform.ts">
<violation number="1" location="packages/sdk/src/internal/detect-platform.ts:80">
P1: Avoid direct `process.version` access in the Edge branch; it can crash platform-header detection when `process` is unavailable in edge runtimes.</violation>
</file>
<file name="packages/sdk/src/internal/utils/values.ts">
<violation number="1" location="packages/sdk/src/internal/utils/values.ts:61">
P2: `validatePositiveInteger` currently accepts `0` because it only rejects values `< 0`, which contradicts the function’s positive-integer contract.</violation>
</file>
<file name="packages/sdk/src/resources/automations.ts">
<violation number="1" location="packages/sdk/src/resources/automations.ts:76">
P2: `run()` is typed as returning `AutomationRun`, but `automation.runNow` returns only `{ automationId, runId }`. This gives consumers an incorrect response contract.</violation>
</file>
<file name="packages/sdk/src/client.ts">
<violation number="1" location="packages/sdk/src/client.ts:301">
P1: Spreading `options` after the relay headers allows `options.headers` to replace the JWT auth headers, which can break relay authentication. Merge caller headers into the relay header object instead of letting the top-level spread overwrite `headers`.</violation>
<violation number="2" location="packages/sdk/src/client.ts:493">
P1: Same spreading order bug as `hostMutation`: caller-supplied `options.headers` would overwrite the JWT auth, and `options.query` would overwrite the tRPC input parameters. Spread `...options` first, then apply the critical fields.</violation>
<violation number="3" location="packages/sdk/src/client.ts:554">
P2: Add an in-flight refresh guard for `_getJwt()` so concurrent host requests await the same token exchange instead of issuing multiple `/api/auth/token` requests.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| : Object.entries(details.headers) | ||
| ).map(([name, value]) => [ | ||
| name, | ||
| name.toLowerCase() === "api_key" || |
Contributor
There was a problem hiding this comment.
P1: Sensitive header redaction misses x-api-key, so API keys can be leaked in logs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk/src/internal/utils/log.ts, line 114:
<comment>Sensitive header redaction misses `x-api-key`, so API keys can be leaked in logs.</comment>
<file context>
@@ -0,0 +1,130 @@
+ : Object.entries(details.headers)
+ ).map(([name, value]) => [
+ name,
+ name.toLowerCase() === "api_key" ||
+ name.toLowerCase() === "authorization" ||
+ name.toLowerCase() === "cookie" ||
</file context>
| 'X-Stainless-OS': 'Unknown', | ||
| 'X-Stainless-Arch': `other:${EdgeRuntime}`, | ||
| 'X-Stainless-Runtime': 'edge', | ||
| 'X-Stainless-Runtime-Version': (globalThis as any).process.version, |
Contributor
There was a problem hiding this comment.
P1: Avoid direct process.version access in the Edge branch; it can crash platform-header detection when process is unavailable in edge runtimes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk/src/internal/detect-platform.ts, line 80:
<comment>Avoid direct `process.version` access in the Edge branch; it can crash platform-header detection when `process` is unavailable in edge runtimes.</comment>
<file context>
@@ -0,0 +1,196 @@
+ 'X-Stainless-OS': 'Unknown',
+ 'X-Stainless-Arch': `other:${EdgeRuntime}`,
+ 'X-Stainless-Runtime': 'edge',
+ 'X-Stainless-Runtime-Version': (globalThis as any).process.version,
+ };
+ }
</file context>
| name ||= getName(value); | ||
|
|
||
| if (!options?.type) { | ||
| const type = parts.find( |
Contributor
There was a problem hiding this comment.
P2: The inferred MIME type is never applied because find(...) returns the whole part object, not its type string.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk/src/internal/to-file.ts, line 127:
<comment>The inferred MIME type is never applied because `find(...)` returns the whole part object, not its `type` string.</comment>
<file context>
@@ -0,0 +1,172 @@
+ name ||= getName(value);
+
+ if (!options?.type) {
+ const type = parts.find(
+ (part) => typeof part === "object" && "type" in part && part.type,
+ );
</file context>
| return (globalThis as any).process.env?.[env]?.trim() || undefined; | ||
| } | ||
| if (typeof (globalThis as any).Deno !== "undefined") { | ||
| return (globalThis as any).Deno.env?.get?.(env)?.trim() || undefined; |
Contributor
There was a problem hiding this comment.
P2: Deno.env.get can throw PermissionDenied without allow-env; this breaks the function contract that inaccessible env vars return undefined.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk/src/internal/utils/env.ts, line 15:
<comment>`Deno.env.get` can throw `PermissionDenied` without `allow-env`; this breaks the function contract that inaccessible env vars return `undefined`.</comment>
<file context>
@@ -0,0 +1,18 @@
+ return (globalThis as any).process.env?.[env]?.trim() || undefined;
+ }
+ if (typeof (globalThis as any).Deno !== "undefined") {
+ return (globalThis as any).Deno.env?.get?.(env)?.trim() || undefined;
+ }
+ return undefined;
</file context>
| if (typeof n !== "number" || !Number.isInteger(n)) { | ||
| throw new SupersetError(`${name} must be an integer`); | ||
| } | ||
| if (n < 0) { |
Contributor
There was a problem hiding this comment.
P2: validatePositiveInteger currently accepts 0 because it only rejects values < 0, which contradicts the function’s positive-integer contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk/src/internal/utils/values.ts, line 61:
<comment>`validatePositiveInteger` currently accepts `0` because it only rejects values `< 0`, which contradicts the function’s positive-integer contract.</comment>
<file context>
@@ -0,0 +1,118 @@
+ if (typeof n !== "number" || !Number.isInteger(n)) {
+ throw new SupersetError(`${name} must be an integer`);
+ }
+ if (n < 0) {
+ throw new SupersetError(`${name} must be a positive integer`);
+ }
</file context>
…tart - @superset/sdk → @superset_sh/sdk in code samples - show organizationId in the constructor (required for org-scoped resources) - update tasks.retrieve example to reflect Task | null return - link to docs.superset.sh/docs/sdk Bumps to 0.0.1-alpha.4 for republish (npm displays the dist/README.md, so the fix only ships once the new version is live).
Contributor
There was a problem hiding this comment.
1 issue found across 7 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/docs/content/docs/sdk/getting-started.mdx">
<violation number="1" location="apps/docs/content/docs/sdk/getting-started.mdx:34">
P2: The getting-started snippet uses the wrong environment variable name for `organizationId` (`SUPERSET_ORG_ID`), which causes org-scoped calls to fail when copied as-is.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Open with what the SDK is and concrete things you can do with it, not 'thin wrapper around the API the CLI uses'. - Recommend staying on latest, not pinning. - Drop runtime-deps / bundle-format trivia that doesn't matter to a user trying to actually use the SDK. - API key + organization id come from the desktop app (Settings → API Keys), not the web URL. - Remove the 'Switching organizations' / withOptions section — the SDK is per-org-scoped and we don't want to advertise cross-org calls. - Strip 'Mirrors superset xxx' annotations from the reference; the SDK stands on its own.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/docs/content/docs/sdk/getting-started.mdx`:
- Around line 116-130: The snippet references client but never creates it;
instantiate a Superset client before the try block so the code is copy/paste
runnable: import Superset (already present) and create a client variable (e.g.,
const client = new Superset(...) or Superset({...}) depending on the SDK
constructor) with minimal config placeholders (base URL and auth/api key or env
vars), then call client.tasks.create inside the try/catch that already checks
RateLimitError, NotFoundError and APIError; ensure the new client instantiation
appears directly before the try so readers can run the example as-is.
- Around line 19-39: The docs claim the SDK works in browsers but then shows
using the long-lived SUPERSET_API_KEY and SUPERSET_ORGANIZATION_ID env vars; add
a clear security warning immediately after the "Everything is fully typed..."
line or at the start of the "Get an API key" section stating that sk_live_* API
keys must never be embedded in frontend/browser code, and recommend server-only
usage: obtain API keys on the backend, use backend-issued short-lived JWTs or a
backend proxy to call the SDK from browser apps, and link to the desktop app/API
Keys instructions for server-side usage; mention SUPERSET_API_KEY and
SUPERSET_ORGANIZATION_ID by name so readers know which values are sensitive.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 32c2a889-97b4-4fba-bd47-e1e8372ba12b
📒 Files selected for processing (2)
apps/docs/content/docs/sdk/getting-started.mdxapps/docs/content/docs/sdk/reference.mdx
✅ Files skipped from review due to trivial changes (1)
- apps/docs/content/docs/sdk/reference.mdx
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/docs/content/docs/sdk/getting-started.mdx">
<violation number="1" location="apps/docs/content/docs/sdk/getting-started.mdx:19">
P2: Add a browser-safety caveat here. As written, this can be read as guidance to use `sk_live_` API keys directly in browser code, which exposes long-lived credentials.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Install block uses fumadocs <Tabs> for npm/pnpm/yarn/bun. - Renamed 'Your first call' → 'Create the client' in getting-started. - New advanced.mdx is a recipes page, not an API escape hatches page. Centerpiece example: Sentry webhook → file a task → spawn a workspace on a target host → dispatch an agent into it. Plus a daily Slack triage digest, a bulk reassign script, and on-demand automation dispatch from external tools.
Contributor
There was a problem hiding this comment.
2 issues found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/docs/content/docs/sdk/advanced.mdx">
<violation number="1" location="apps/docs/content/docs/sdk/advanced.mdx:34">
P1: Verify webhook authenticity before processing Sentry payloads; this handler currently trusts any POST body and can be abused to trigger task/workspace/automation actions.</violation>
<violation number="2" location="apps/docs/content/docs/sdk/advanced.mdx:70">
P2: This RRULE is recurring annually, so the example can re-run in future years despite the comment saying it never re-runs.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const PROJECT_ID = process.env.SUPERSET_PROJECT_ID!; // projects.list() to find yours | ||
|
|
||
| app.post('/webhooks/sentry', async (c) => { | ||
| const event = await c.req.json(); |
Contributor
There was a problem hiding this comment.
P1: Verify webhook authenticity before processing Sentry payloads; this handler currently trusts any POST body and can be abused to trigger task/workspace/automation actions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/docs/content/docs/sdk/advanced.mdx, line 34:
<comment>Verify webhook authenticity before processing Sentry payloads; this handler currently trusts any POST body and can be abused to trigger task/workspace/automation actions.</comment>
<file context>
@@ -0,0 +1,149 @@
+const PROJECT_ID = process.env.SUPERSET_PROJECT_ID!; // projects.list() to find yours
+
+app.post('/webhooks/sentry', async (c) => {
+ const event = await c.req.json();
+ const { title, web_url: sentryUrl, culprit } = event.data.issue;
+ const slug = `sentry-${event.data.issue.short_id.toLowerCase()}`;
</file context>
Adds an optional `agents: [{ prompt }]` array to workspaces.create. When
provided, the SDK creates the worktree, then dispatches each agent into
it via a one-shot automation (yearly rrule = effectively single-run).
Returns a CreatedWorkspace = HostWorkspace + { agentRuns: AutomationRun[] }
so callers can correlate runs back to the spawned agents.
Sugar over existing primitives — same effect as the old
create-workspace + create-automation + run sequence, but the recipe in
the docs collapses from three calls to one. Updates the Sentry-webhook
example accordingly.
Contributor
There was a problem hiding this comment.
3 issues found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/sdk/src/resources/workspaces.ts">
<violation number="1" location="packages/sdk/src/resources/workspaces.ts:74">
P1: Default agent config is created without `enabled: true`, which can cause newly spawned agent runs to fail as disabled.</violation>
<violation number="2" location="packages/sdk/src/resources/workspaces.ts:76">
P2: Forward `RequestOptions` to `automation.create` so cancellation/timeout/header overrides apply to the whole `workspaces.create` flow.</violation>
<violation number="3" location="packages/sdk/src/resources/workspaces.ts:92">
P2: Forward `RequestOptions` to `automation.runNow` to keep request controls consistent within `workspaces.create`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…, flag the gap
The 'agents: [{ prompt }]' shorthand is the API surface we want to land
on. Today it sends a minimal agentConfig that the host service can't
actually launch from. Documented honestly with a TODO until we bake in
defaults for built-in agent presets.
The SDK defaults to claude when agent is omitted, but having it visible in the docs makes the shape of an agent spawn entry obvious at a glance.
Contributor
There was a problem hiding this comment.
3 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/docs/content/docs/sdk/advanced.mdx">
<violation number="1" location="apps/docs/content/docs/sdk/advanced.mdx:15">
P3: The docs example can throw when no automations exist because it dereferences `[0].agentConfig` without checking for an empty list.</violation>
</file>
<file name="apps/docs/content/docs/sdk/reference.mdx">
<violation number="1" location="apps/docs/content/docs/sdk/reference.mdx:123">
P2: The docs recommend `(await client.automations.list())[0].agentConfig` without guarding empty results, which can throw for users with no automations.</violation>
<violation number="2" location="apps/docs/content/docs/sdk/reference.mdx:133">
P2: The example contradicts the new warning: it still uses shorthand `agent` entries without `agentConfig`, so it follows the minimal-config path that may not actually launch agents.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| Create a worktree on a specific host. Optionally spawn one or more agents inside it as soon as the worktree is ready. Goes through the relay tunnel — the host must be online. | ||
|
|
||
| <Callout type="info"> | ||
| **TODO** — the `agents` shorthand is the surface we're aiming for. The SDK currently sends a minimal `agentConfig: { id: 'claude', kind: 'terminal' }`; the host service still needs full launch info (`command`, `promptCommand`, …) to actually start the agent. We'll bake in defaults for built-in presets in a follow-up. Until then, pass a full `agentConfig` (crib one from `(await client.automations.list())[0].agentConfig`). |
Contributor
There was a problem hiding this comment.
P2: The docs recommend (await client.automations.list())[0].agentConfig without guarding empty results, which can throw for users with no automations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/docs/content/docs/sdk/reference.mdx, line 123:
<comment>The docs recommend `(await client.automations.list())[0].agentConfig` without guarding empty results, which can throw for users with no automations.</comment>
<file context>
@@ -119,15 +119,19 @@ Returns `Array<Workspace>`.
Create a worktree on a specific host. Optionally spawn one or more agents inside it as soon as the worktree is ready. Goes through the relay tunnel — the host must be online.
+<Callout type="info">
+**TODO** — the `agents` shorthand is the surface we're aiming for. The SDK currently sends a minimal `agentConfig: { id: 'claude', kind: 'terminal' }`; the host service still needs full launch info (`command`, `promptCommand`, …) to actually start the agent. We'll bake in defaults for built-in presets in a follow-up. Until then, pass a full `agentConfig` (crib one from `(await client.automations.list())[0].agentConfig`).
+</Callout>
+
</file context>
Suggested change
| **TODO** — the `agents` shorthand is the surface we're aiming for. The SDK currently sends a minimal `agentConfig: { id: 'claude', kind: 'terminal' }`; the host service still needs full launch info (`command`, `promptCommand`, …) to actually start the agent. We'll bake in defaults for built-in presets in a follow-up. Until then, pass a full `agentConfig` (crib one from `(await client.automations.list())[0].agentConfig`). | |
| **TODO** — the `agents` shorthand is the surface we're aiming for. The SDK currently sends a minimal `agentConfig: { id: 'claude', kind: 'terminal' }`; the host service still needs full launch info (`command`, `promptCommand`, …) to actually start the agent. We'll bake in defaults for built-in presets in a follow-up. Until then, pass a full `agentConfig` (for example, copy one from an existing automation after confirming the list is non-empty). |
Comment on lines
+133
to
+134
| { agent: 'claude', prompt: 'Implement the auth flow described in SUPER-100.' }, | ||
| { agent: 'claude', prompt: 'Write integration tests for the new auth flow.' }, |
Contributor
There was a problem hiding this comment.
P2: The example contradicts the new warning: it still uses shorthand agent entries without agentConfig, so it follows the minimal-config path that may not actually launch agents.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/docs/content/docs/sdk/reference.mdx, line 133:
<comment>The example contradicts the new warning: it still uses shorthand `agent` entries without `agentConfig`, so it follows the minimal-config path that may not actually launch agents.</comment>
<file context>
@@ -119,15 +119,19 @@ Returns `Array<Workspace>`.
agents: [ // optional — spawn agents on creation
- { prompt: 'Implement the auth flow described in SUPER-100.' },
- { prompt: 'Write integration tests for the new auth flow.', agent: 'claude' },
+ { agent: 'claude', prompt: 'Implement the auth flow described in SUPER-100.' },
+ { agent: 'claude', prompt: 'Write integration tests for the new auth flow.' },
],
</file context>
Suggested change
| { agent: 'claude', prompt: 'Implement the auth flow described in SUPER-100.' }, | |
| { agent: 'claude', prompt: 'Write integration tests for the new auth flow.' }, | |
| { | |
| prompt: 'Implement the auth flow described in SUPER-100.', | |
| agentConfig: { id: 'claude', kind: 'terminal', command: '<command>', promptCommand: '<promptCommand>' }, | |
| }, | |
| { | |
| prompt: 'Write integration tests for the new auth flow.', | |
| agentConfig: { id: 'claude', kind: 'terminal', command: '<command>', promptCommand: '<promptCommand>' }, | |
| }, |
| ## Sentry webhook → agent on the bug | ||
|
|
||
| <Callout type="info"> | ||
| **TODO** — the `agents: [{ prompt }]` shorthand below is the API we're aiming for. Today the SDK sends a minimal `agentConfig: { id: 'claude', kind: 'terminal' }`; the host service still needs full launch info (`command`, `promptCommand`, …) to actually start the agent. We'll bake in defaults for built-in presets in a follow-up. Until then you can pass a full `agentConfig` per agent — crib one from `(await client.automations.list())[0].agentConfig`. |
Contributor
There was a problem hiding this comment.
P3: The docs example can throw when no automations exist because it dereferences [0].agentConfig without checking for an empty list.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/docs/content/docs/sdk/advanced.mdx, line 15:
<comment>The docs example can throw when no automations exist because it dereferences `[0].agentConfig` without checking for an empty list.</comment>
<file context>
@@ -11,6 +11,10 @@ The SDK's individual methods are simple. The interesting part is what you compos
## Sentry webhook → agent on the bug
+<Callout type="info">
+**TODO** — the `agents: [{ prompt }]` shorthand below is the API we're aiming for. Today the SDK sends a minimal `agentConfig: { id: 'claude', kind: 'terminal' }`; the host service still needs full launch info (`command`, `promptCommand`, …) to actually start the agent. We'll bake in defaults for built-in presets in a follow-up. Until then you can pass a full `agentConfig` per agent — crib one from `(await client.automations.list())[0].agentConfig`.
+</Callout>
+
</file context>
Suggested change
| **TODO** — the `agents: [{ prompt }]` shorthand below is the API we're aiming for. Today the SDK sends a minimal `agentConfig: { id: 'claude', kind: 'terminal' }`; the host service still needs full launch info (`command`, `promptCommand`, …) to actually start the agent. We'll bake in defaults for built-in presets in a follow-up. Until then you can pass a full `agentConfig` per agent — crib one from `(await client.automations.list())[0].agentConfig`. | |
| **TODO** — the `agents: [{ prompt }]` shorthand below is the API we're aiming for. Today the SDK sends a minimal `agentConfig: { id: 'claude', kind: 'terminal' }`; the host service still needs full launch info (`command`, `promptCommand`, …) to actually start the agent. We'll bake in defaults for built-in presets in a follow-up. Until then you can pass a full `agentConfig` per agent — for example, read the first automation safely: `const first = (await client.automations.list())[0]; if (!first) throw new Error('Create an automation first'); const agentConfig = first.agentConfig;`. |
The old 'daily triage' / 'bulk reassign' recipes didn't show off agents
at all — pure CRUD that you could do in any client. Replaced with two
flows that actually use the SDK's killer feature:
1. Burn down the P0 backlog overnight — list every urgent task and spawn
a Claude agent in parallel on each one, comes back with a branch per
bug.
2. Sweep a refactor across every repo — list projects, fan out one agent
per repo to do an internal API rename / dep bump / lint rule rollout.
Same shape works for 'bump TS to 5.7', 'swap Sentry SDK', etc.
Both lean into Promise.all + workspaces.create({ agents }) so the fanout
shape is the visible thing. On-demand dispatch callout retained as the
final note.
CI:
- Drop empty 'dependencies: {}' from packages/sdk/package.json (sherif).
Real bugs:
- automations.run() return type was wrong — server returns
{ automationId, runId }, not the full AutomationRun row. Introduced
AutomationRunDispatched and updated workspaces.create's agentRuns
array to use it.
- hostMutation/hostQuery were spreading ...options after the relay-auth
headers, so a caller passing options.headers would silently strip
the JWT. Now: options first, then auth headers force-merged via
buildHeaders so caller-provided headers can't drop the JWT.
- _getJwt could thundering-herd on a cold cache (N concurrent host
calls = N /api/auth/token requests). Added an in-flight promise
guard so concurrent callers share a single exchange.
Docs:
- getting-started: explicit warning not to ship sk_live_ keys to the
browser; recommend a thin server proxy.
- Smoke header comment uses the published package name.
Bumps to 0.0.1-alpha.6.
Contributor
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
- workspaces.create: forward RequestOptions through to internal automation.create / automation.runNow calls (Cubic P2) - workspaces.create: default agentConfig now includes 'enabled: true' to match the shape prod automations have (Cubic P1) - docs: error-handling snippet now declares 'const client = new Superset()' so it copy-pastes cleanly (CodeRabbit Minor) - docs: docs reference workspace.agentRuns[].runId (was .id which doesn't exist on AutomationRunDispatched) - docs: prose note about Sentry signature verification before code block, kept out of the example to not bloat it (Cubic P1)
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.
Summary
New `packages/sdk` — external-client-facing TypeScript SDK that wraps the same tRPC procedures the CLI uses. Published as `@superset_sh/sdk@0.0.1-alpha.3`.
Verification
End-to-end against prod (`api.superset.sh`):
Test plan
Summary by CodeRabbit
New Features
Documentation
Chores