Skip to content

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

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

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

Conversation

@saileshwar-skyflow

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

… 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

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

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.

…/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

Copy link
Copy Markdown

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

@github-actions

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.

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

Copy link
Copy Markdown

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

@github-actions

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.

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

Copy link
Copy Markdown

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

@github-actions

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

…/data

Aligns the Node flowvault insert/update contract with the Java/Python SDKs:
- InsertRecord/UpdateRecord gain an optional per-record `table`; when any
  record sets it, a single insert/update call targets multiple tables (the
  generated InsertRecordData/UpdateRecordData already carry tableName).
- Insert/Update response records now expose `tokens` and `data` separately
  instead of a single merged `fields`.

Updated unit tests (incl. multi-table coverage) and the api-report snapshot.
flowvault 156 tests pass (100% lines, branches 96.97%); common 232, skyvault
878 unchanged.

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

Copy link
Copy Markdown

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

@github-actions

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
saileshwar-skyflow force-pushed the saileshwar/SK-3225-add-flowvault-node-sdk branch from 1de5bec to 874487c Compare October 18, 2026 15:44
@github-actions

Copy link
Copy Markdown

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

@github-actions

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.

@github-actions

Copy link
Copy Markdown

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

@github-actions

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.

Expand flowvault/README.md into a full SDK guide (overview, install, auth,
client init, all 9 operations, custom headers, error handling) and add
flowvault/samples/ — one runnable TypeScript example per operation under
vault-api/, plus service-account token-generation samples — mirroring the
skyflow-flowvault-python samples. All samples type-check in strict mode against
the built SDK types.

flowvault-only; skyvault and common untouched.

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

Copy link
Copy Markdown

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

@github-actions

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.

Config (parity with python flowvault, limited to what the Node generated client
can honor):
- VaultConfig gains optional timeout (seconds), maxRetries, and vaultUrl;
  the flowvault VaultClient forwards timeout/maxRetries as the generated
  request's timeoutInSeconds/maxRetries, and vaultUrl overrides the derived URL.
  (connect/read/write timeouts and retry-delay knobs are fixed in generated code.)

Custom headers:
- options.setRequestHeaders(...) is now actually wired — merged into every
  operation's request headers. Added the missing options params to delete,
  query, getTokens, uploadFiles, deleteFiles so headers work on all 9 ops.

Samples/README updated; added tests (162 pass, 100% stmts/lines). flowvault +
the shared VaultConfig only; skyvault untouched; api-report unchanged.

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

Copy link
Copy Markdown

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

@github-actions

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.

…t fields

Closes two parity gaps vs the Java/Python flowvault SDKs (generated client
already supports both; hand-written layer only):
- InsertRecord gains optional per-record `upsert`; when any record sets it, each
  record sends its own upsert (falling back to the request-level one) and the
  request-level upsert is dropped.
- GetRecord (multi-table get) gains optional `columns` / `columnRedactions` /
  `uniqueValues`, wired per record.

Added tests (164 pass, 100% lines); api-report regenerated. flowvault-only;
skyvault and common untouched.

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

Copy link
Copy Markdown

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

@github-actions

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.

- UpdateRequest gains an optional request-level `updateType` ("UPDATE" |
  "REPLACE"), wired to the generated UpdateRequest.updateType (no generated
  change needed). Closes the last parity gap vs Java/Python flowvault.
- README + samples updated to match the python flowvault SDK: per-record
  `upsert` on insert, multi-table get per-record columns/redactions/uniqueValues,
  and update `updateType`.

166 tests pass (100% lines); api-report regenerated; samples type-check.
flowvault-only; skyvault/common untouched.

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

Copy link
Copy Markdown

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

@github-actions

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.

FlowVault (v3) is served from {clusterId}.skyvault.skyflowapis.{dev|com|
-preview.com|tech}, a different subdomain than the shared getVaultURL
(vault.*, used by skyvault/v2). The facade was reusing getVaultURL, so every
call 404'd. Added a flowvault-local getFlowVaultURL with the four confirmed
skyvault.* hosts and used it in buildVaultEntry.

flowvault-only; skyvault/common untouched.

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

Copy link
Copy Markdown

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

@github-actions

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.

…or extraction

Two bugs surfaced running the e2e against the live vault:
- Insert/update sent tableName at BOTH the request level and per-record when any
  record carried its own table, which the API rejects (400 "table name ... at
  one place"). Now the request-level tableName is omitted whenever per-record
  tables are used (table lives in exactly one place), matching Java/Python.
- A per-record failure returned with a 4xx HTTP status (e.g. delete/get/update
  of a non-existent id -> 404 whose body carries a records/response array) was
  thrown as a whole-call error. Now those per-record records are surfaced as a
  normal response (via unaryError), and genuine whole-call errors still throw.

Added error-extraction tests for insert/get/update/delete/detokenize/getTokens/
deleteFiles. 174 tests pass (99.4% lines). flowvault-only; skyvault/common untouched.

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

Copy link
Copy Markdown

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

@github-actions

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.

validateGetRequest rejected empty ids even for a get-by-unique-value, where the
lookup is by GetOptions.setUniqueValues (not ids). It now accepts a single-table
get with empty ids as long as uniqueValues are provided on the options
("ids OR uniqueValues", matching Java/Python flowvault).

175 tests pass. flowvault-only; skyvault/common untouched; api-report unchanged.

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

Copy link
Copy Markdown

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

@github-actions

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant