Skip to content

SK-3225: Split SDK into common / skyvault / flowvault; add FlowVault Node SDK - #348

Open
saileshwar-skyflow wants to merge 4 commits into
mainfrom
saileshwar/SK-3225-add-flowvault-node-sdk
Open

SK-3225: Split SDK into common / skyvault / flowvault; add FlowVault Node SDK#348
saileshwar-skyflow wants to merge 4 commits into
mainfrom
saileshwar/SK-3225-add-flowvault-node-sdk

Conversation

@saileshwar-skyflow

@saileshwar-skyflow saileshwar-skyflow commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator

SK-3225 — Split the SDK into common / skyvault / flowvault and add the FlowVault Node SDK

Restructures the single skyflow-node package into an npm-workspaces monorepo mirroring the Java SDK split, and adds skyflow-flowvault-node as a second, independently-published product built from the same codebase.

Modules

Workspace npm package Version line Role
common — (private, bundled into each package) internal Contract-agnostic core: errors, utils, jwt, logging, service-account auth, config/credentials, generic validations, mutable SDK identity, Base* request/response/options classes, BaseSkyflow facade base, and the generated auth client. Builds & tests in isolation.
skyvault skyflow-node 2.x (unchanged) The existing contract — vault data plane + connections + detect. Public API byte-for-byte unchanged (74 exports). Its Skyflow/models extend the common Base* classes; keeps its data-plane generated client.
flowvault skyflow-flowvault-node 1.x (new) New unary v2 contract: insert/get/update/delete/query/detokenize/getTokens/uploadFiles/deleteFiles, wired to the generated v2 client; reconciled against the Java/Python flowvault SDKs.

How a product is selected

Solely by the installed npm package — npm i skyflow-node vs npm i skyflow-flowvault-node. No build flags, runtime switches, or env config.

Packaging & identity

  • Each published package bundles common inline (tsup, dist/) so the tarball is self-contained — no skyflow-common dependency ships (verified via npm pack + install into a fresh consumer; require() works for both).
  • stripInternal keeps internals out of the published .d.ts; only the public entry is exposed via exports.
  • A mutable SDK identity (setSdkIdentity) in common, stamped per layer from an in-source version.ts, makes each product self-report its own name@version in telemetry and error text (skyvault v2.1.2, flowvault v1.0.0).

Versioning & CI

  • Independent tags: plain semver (x.y.z) → publishes skyflow-node; prefixed (flowvault/x.y.z) → publishes skyflow-flowvault-node. Internal/JFrog releases routed per branch namespace.
  • scripts/bump_version.sh <product> <version> [sha] bumps the right package.json + version.ts.
  • One command builds + tests both SDKs (npm run build / npm test fan out across the workspace).
  • Per-package API contract snapshots (api-extractor) gate the public surface of both SDKs (contract-tests.yml).

Tests

  • common 232, skyvault 878 (backward-compat preserved), flowvault 132 (~99.85% statement coverage) — all green.

Maintainer notes (flagged, not fixed — generated code untouched)

  • Both common and flowvault generated clients ship a top-level Client.ts (patchV2VaultsId) that fails strict (encodeURIComponent on a possibly-undefined vaultId); unused by the hand-written layers (which compose resource clients directly), so it + its index.ts are excluded in the relevant tsconfigs — a regeneration concern.
  • Legacy skyvault generated dir keeps its historical leading-space name ( _generated_), preserved as-is.

Full architecture, versioning, release flow and flags: docs/sdk-split-plan.md.

🤖 Generated with Claude Code

@saileshwar-skyflow @claude
SK-3225: Split SDK into common/skyvault/flowvault; add FlowVault Node…
34cda0c 
… SDK

Restructure the single `skyflow-node` package into an npm-workspaces monorepo
mirroring the Java SDK split (common / skyvault / flowvault).

- common (private, bundled into each published package): the contract-agnostic
  core — errors, utils, jwt, logging, service-account auth, config/credentials,
  generic validations, mutable SDK identity, Base* request/response/options
  classes, the BaseSkyflow facade base, and the generated auth client. Builds
  and tests in isolation.
- skyvault -> skyflow-node (2.x): the existing contract, public API unchanged
  (74 exports, byte-for-byte). Its Skyflow/models extend the common Base*
  classes; keeps connections, detect and its data-plane generated client.
- flowvault -> skyflow-flowvault-node (1.x): new unary v2 contract
  (insert/get/update/delete/query/detokenize/getTokens/uploadFiles/deleteFiles)
  wired to the generated v2 client; reconciled with the Java/Python flowvault
  SDKs.

Each layer re-exports common and bundles it (tsup) into a self-contained
tarball (verified via npm pack); the mutable SDK identity makes each product
self-report its own name@version. Independent versioning: plain-semver tags
publish skyflow-node, flowvault/* tags publish skyflow-flowvault-node.

Tests: common 232, skyvault 878 (backward compatibility preserved), flowvault
132 (~99.85% coverage). Per-package api-extractor contract snapshots gate the
public API of both SDKs. PR CI builds and tests both SDKs with one command.

Docs: docs/sdk-split-plan.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 14, 2026

Copy link
Copy Markdown

🔐 Gitleaks Findings: 2 issue(s) detected

🔸 Rule: jwt
📄 File: common/src/_generated_/rest/api/resources/authentication/client/requests/V1GetAuthTokenRequest.ts:null
📝 Description: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
🔑 Secret: **********
🔗 Path: common/src/generated/rest/api/resources/authentication/client/requests/V1GetAuthTokenRequest.ts:null

🔸 Rule: jwt
📄 File: common/src/_generated_/rest/api/resources/authentication/client/Client.ts:null
📝 Description: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
🔑 Secret: **********
🔗 Path: common/src/generated/rest/api/resources/authentication/client/Client.ts:null

@github-actions

github-actions Bot commented Oct 14, 2026

Copy link
Copy Markdown

Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging.

@saileshwar-skyflow @claude
SK-3225: fix PR CI (eslint ignores + gitleaks allowlist for generated…
1fd6971 
…/dist)

- eslint.config.mjs: ignore all generated dirs (**/_generated_/**, legacy
  leading-space ** / _generated_/**), all workspace test dirs (**/test/**),
  and build output across workspaces — the old ignores only matched the
  single-package paths.
- Replace three banned magic-string comparisons in hand-written code
  (typeof === "symbol"/"function") with equivalent non-literal checks.
- Rule/gitleaks.toml: allowlist the maintainer-owned generated dirs (example
  JWTs) and the bundled dist/ output.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 14, 2026

Copy link
Copy Markdown

✅ Gitleaks Findings: No secrets detected. Safe to proceed!

@github-actions

github-actions Bot commented Oct 14, 2026

Copy link
Copy Markdown

Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging.

@saileshwar-skyflow @claude
SK-3225: fix skyvault jest — don't babel-transform compiled common/lib
73d319a 
CI resolves the skyflow-common symlink to its realpath (outside node_modules),
so babel-jest was transforming the compiled common/lib and injecting core-js
(an undeclared dep) -> suites failed to run. Add `common/lib` to
transformIgnorePatterns so the already-CJS common package is loaded as-is.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 14, 2026

Copy link
Copy Markdown

✅ Gitleaks Findings: No secrets detected. Safe to proceed!

@github-actions

github-actions Bot commented Oct 14, 2026

Copy link
Copy Markdown

Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging.

@saileshwar-skyflow @claude
SK-3225: add flowvault branch-coverage tests to clear codecov/project
874487c 
Covers the defensive edge branches the codecov/project gate flagged
(net-new flowvault code dipped overall coverage 0.44% vs base):
- empty/absent `records`/`response` bodies map to [] across all ops
- per-record errors with no x-request-id header -> requestId null
- getRequestId with an absent raw response
- uploadFiles: non-Error PUT rejection, no-name/trailing-dot content-type,
  response record with no matching prepared columns
- VaultClient: default log level, unrecognized cached auth type

flowvault branches 92.51% -> 96.89% (statements/lines 100%); 132 -> 153 tests.
No source/generated/config changes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 14, 2026

Copy link
Copy Markdown

✅ Gitleaks Findings: No secrets detected. Safe to proceed!

@github-actions

github-actions Bot commented Oct 14, 2026

Copy link
Copy Markdown

Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging.

@promptless

promptless Bot commented Oct 15, 2026

Copy link
Copy Markdown

Promptless documentation updates

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@saileshwar-skyflow