feat(sdk/js): align with Rust SDK and guest-agent proto#690
Open
Leechael wants to merge 13 commits into
Open
Conversation
align with Rust SDK (Option<String>) and Python SDK (str | None) where path defaults to empty string when not provided.
proto AppInfo gained these fields in dstack 0.5.7 (commit 63f30ce). Mark them optional so callers on older guest-agent versions still parse cleanly.
replace crypto.createHash('sha384'|'sha256') calls with @noble/hashes
sha384/sha256, which work identically in node and browsers without
needing a polyfill. @noble/hashes is already a peer dependency.
this prepares for removing crypto-browserify, whose upstream chain
(elliptic, create-ecdh, browserify-sign) carries unpatched advisories.
remove the crypto-browserify dep and the package.json browser field that aliased node's crypto to it. the only consumers of node crypto in this SDK already moved to @noble/hashes in the previous commit. this removes the elliptic / create-ecdh / browserify-sign / bn.js (4.x) transitive chain whose advisories have no upstream fix.
bun.lockb pinned bn.js@5.2.2, which is below the patched 5.2.3 for advisory GHSA-378v-28hj-76wf (infinite loop). @solana/web3.js@1.98.4 declares `bn.js: ^5.2.1`, so a fresh resolution picks 5.2.3 naturally. incidentally migrates to bun's text-format `bun.lock` (default since bun 1.2), replacing the binary `bun.lockb`. The text format is diffable in git.
Leechael
force-pushed
the
feat/sdk-js-api-parity
branch
from
7fe8c3d to
2ed5015
Compare
move @solana/web3.js, viem, and @noble/curves out of optionalDependencies into peerDependencies (with optional meta). they were auto-installed by npm despite the "optional" label, dragging ~70 transitive packages into every downstream install. now a bare `npm install @phala/dstack-sdk` pulls only @noble/hashes. consumers that import /solana, /viem, /encrypt-env-vars, or /verify-env-encrypt-public-key submodules must install the matching peer explicitly.
Leechael
force-pushed
the
feat/sdk-js-api-parity
branch
from
4794930 to
a485ec8
Compare
add a \"files\" allowlist so npm pack ships only README, LICENSE, package.json, and dist/. drops .npmignore (replaced by the allowlist). before: 60 files / 230 KB unpacked / 57 KB tarball, including bun.lock, src/*.ts, test-outputs.js, tsconfig*.json, and .claude/settings.local.json. after: 43 files / 133 KB unpacked / 30 KB tarball — ~47% smaller, no extraneous files leaking out of the repo.
reasons for rewrite: - npm install instructions did not mention peer deps (consumers now need @noble/hashes explicitly, plus per-submodule peers after the 0.5.8 optionalDependencies → peerDependencies change) - getKey signature was stale (missing algorithm arg, path shown as required, key return type listed as hex string instead of Uint8Array) - sign/verify section said "not yet released" though shipped in 0.5.7 - blockchain helpers used the deprecated toViemAccount / toKeypair variants instead of toViemAccountSecure / toKeypairSecure - no coverage of attest, version, info.cloud_vendor / cloud_product, getTlsKey notBefore / notAfter / withAppInfo, secp256k1_prehashed, AppCompose type, or the verify+encrypt env-var flow - migration content for the deprecated TappdClient surfaced ahead of the actual current API structure: one section per capability area (keys, attestation, sign & verify, diagnostics, blockchain, compose hash, encrypted env vars), single example per method, compatibility table mapping each feature to the minimum guest agent version, migration section moved to the bottom. length drops from 508 to 291 lines with broader and more accurate coverage. no AI-slop "Use Cases" / "Key Characteristics" filler.
after moving to @noble/hashes and @noble/curves the .browser.ts variants were 95% identical to the node ones — only Buffer vs manual byte helpers and \"import crypto\" vs globalThis.crypto. rewrite both subpaths to use the universal pattern and drop the duplicates in the next commit. - encrypt-env-vars.ts: drop \`import crypto from 'crypto'\` and use the global Web Crypto (works on Node 18+ and browsers) - verify-env-encrypt-public-key.ts: drop Buffer (use manual hex / bigint helpers); make the function bodies stay synchronous like the previous node version, so the public signature does not change both still return the same types and behave identically.
replace the two tsc invocations (node + browser) with a single tsup build that emits both formats from one entry list. - format: cjs (.js) and esm (.mjs); types as .d.ts and .d.mts - splitting/sourcemap off; treeshake on - exports use explicit \"types\"/\"import\"/\"require\" conditions per subpath so TypeScript picks .d.mts for ESM consumers and .d.ts for CommonJS, and runtime picks .mjs vs .js accordingly - drop tsconfig.node.json and tsconfig.browser.json — tsup carries its own config, the root tsconfig.json is still used for typecheck - drop .js.map / .d.ts.map outputs; the previous maps pointed at src/*.ts paths that the package did not ship, so they were dead weight published package shape vs current 0.5.8-beta.1 on npm: files : 60 → 27 unpacked: 230 KB → 107 KB (-54%) tarball : 57 KB → 23 KB (-60%) now also ships ESM (.mjs) — bundlers get tree-shaking, modern Node can \`import\` natively; legacy \`require()\` still works through the .js files.
5 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the JS SDK surface to match the Rust SDK and the guest-agent agent_rpc.proto (notably dstack OS ≥ 0.5.7 additions), while also modernizing the JS build/distribution and removing Node crypto usage in favor of @noble/* and Web Crypto–compatible code paths.
Changes:
- Aligns APIs/types with newer guest-agent proto fields:
InfoResponse.cloud_vendor/cloud_product,TlsKeyOptions.notBefore/notAfter/withAppInfo, and makesgetKey(path)defaultpathto''. - Adds version-gating for new TLS key options by probing the
VersionRPC. - Migrates JS SDK packaging to
tsup, removes browser-specific*.browser.tsimplementations, and switches hashing to@noble/hashes.
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/js/tsup.config.ts | Adds tsup build configuration for multi-entry CJS/ESM output. |
| sdk/js/tsconfig.node.json | Removed old Node-only build config (previous tsc build path). |
| sdk/js/tsconfig.browser.json | Removed old browser-only build config (previous split build path). |
| sdk/js/src/index.ts | Adds new optional info fields, TLS option gating, getKey default path, and switches SHA-384 hashing implementation. |
| sdk/js/src/viem.ts | Switches SHA-256 hashing from Node crypto to @noble/hashes. |
| sdk/js/src/solana.ts | Switches SHA-256 hashing from Node crypto to @noble/hashes. |
| sdk/js/src/get-compose-hash.ts | Switches compose hashing to @noble/hashes + TextEncoder. |
| sdk/js/src/encrypt-env-vars.ts | Uses Web Crypto (crypto.subtle) + noble curves for env var encryption (browser-compatible). |
| sdk/js/src/verify-env-encrypt-public-key.ts | Consolidates verifier implementation (browser-compatible), refactors helpers. |
| sdk/js/src/..browser.ts | Removes browser-specific variants in favor of single shared implementations. |
| sdk/js/README.md | Updates docs for new APIs, peers, compatibility, and usage examples. |
| sdk/js/package.json | Switches to tsup build and updates exports/peers; version bump beta. |
| sdk/js/bun.lock | Adds lockfile for bun-based installs/builds. |
| sdk/js/.npmignore | Removed (publish scope now controlled via files: ["dist"]). |
Comments suppressed due to low confidence (1)
sdk/js/src/index.ts:10
index.tsre-exportsverify-env-encrypt-public-key, which imports@noble/curves. Because this is a static re-export, consumers importing the root entry (@phala/dstack-sdk) will fail at load time if@noble/curvesis not installed, even if they never call the verify functions. This contradicts marking@noble/curvesas an optional peer dependency.
export { getComposeHash } from './get-compose-hash'
export { verifyEnvEncryptPublicKey, verifyEnvEncryptPublicKeyLegacy } from './verify-env-encrypt-public-key'
export type { VerifyOptions } from './verify-env-encrypt-public-key'
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+93
to
+96
| const prefix = new TextEncoder().encode("dstack-env-encrypt-pubkey") | ||
| const separator = new TextEncoder().encode(":") | ||
| const appIdBytes = hexToBytes(appId) | ||
| const timestampBytes = bigintToBeBytes(ts, 8) |
Comment on lines
+118
to
+121
| const prefix = new TextEncoder().encode("dstack-env-encrypt-pubkey") | ||
| const separator = new TextEncoder().encode(":") | ||
| const appIdBytes = hexToBytes(appId) | ||
| const message = concat(prefix, separator, appIdBytes, publicKey) |
Comment on lines
67
to
68
| "test": "vitest", | ||
| "test:ci": "vitest --run", |
| "build": "npm run build:node && npm run build:browser", | ||
| "build:node": "tsc -p tsconfig.node.json", | ||
| "build:browser": "tsc -p tsconfig.browser.json", | ||
| "build": "tsup", |
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
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
Aligns the JS SDK surface with the Rust SDK and the
agent_rpc.protodefinitions inguest-agent/.getKey(path?, ...)—pathnow defaults to''so callers can omit it, matching Rust (Option<String>) and Python (str | None).InfoResponsenow surfacescloud_vendorandcloud_product(optional). Proto AppInfo gained these in dstack 0.5.7 (63f30ce7).TlsKeyOptionsnow acceptsnotBefore,notAfter,withAppInfo. ProtoGetTlsKeyArgsgained these in dstack 0.5.7 (029f167f,c6d1d1ba).Version compatibility
undefined— no runtime check needed.getTlsKeyfirst probes the guest-agent withVersion()(reusing the same gating pattern asensureAlgorithmSupported). On dstack OS < 0.5.7 the Version RPC is unavailable, so the client throws a clear error instead of silently dropping the options.Stacked PR
Base is
feat/sdk-release-workflows(PR #686) — that branch still has the JS SDK beta version bump and the trusted-publisher release workflows.Test plan
npx tsc -p tsconfig.node.json --noEmitpassesnpx tsc -p tsconfig.browser.json --noEmitpassesnpm testagainst a running dstack simulatornotBefore/notAfter/withAppInfoare accepted by guest-agent 0.5.7+