Skip to content

feat!: convert SDK to TypeScript with a dual ESM/CJS build - #243

Open
matt-evervault wants to merge 6 commits into
claude/sdk-cleanup-dead-codefrom
claude/sdk-typescript
Open

feat!: convert SDK to TypeScript with a dual ESM/CJS build#243
matt-evervault wants to merge 6 commits into
claude/sdk-cleanup-dead-codefrom
claude/sdk-typescript

Conversation

@matt-evervault

Copy link
Copy Markdown
Contributor

Why

Modernize the SDK by converting it to TypeScript. This improves type-safety for consumers and maintainers and ships proper bundled type declarations, while keeping the public API and on-the-wire behaviour identical.

Stacked on top of claude/sdk-cleanup-dead-code — review that PR first. This PR's diff is against the cleanup branch.

How

  • Rewrote lib/*.jslib/*.ts in strict TypeScript, preserving runtime behaviour and the byte-exact encryption formats (only module syntax and type annotations changed).
  • Build with tsup to dist/ as dual ESM (index.mjs) + CommonJS (index.js) with bundled .d.ts; repointed main/module/types/exports. require('@evervault/sdk') still returns the EvervaultClient class, and import Evervault from '@evervault/sdk' works for ESM consumers (keepNames preserves error.type).
  • Monkey-patched Node core modules (https.request, tls.*) use default imports so the mutable module.exports is patched — works in both the CJS and ESM builds.
  • Folded the hand-written types.d.ts / domainTargets.d.ts into source types.
  • Ran the existing mocha suite against the TS source via tsx and replaced rewire (incompatible with compiled TS) with proxyquire / shared-singleton config mutation. 204 passing, unchanged from the JS baseline.
  • CI: added typecheck + build steps; bumped CodeQL to v3 with the javascript-typescript language.

Released as a major (7.0) via changeset only because the package's internal file layout and exports map changed.

🤖 Generated with Claude Code

https://claude.ai/code/session_011HwoTRLdV2YMub47j88mv5


Generated by Claude Code

Rewrite the SDK source (lib/*.js -> lib/*.ts) in strict TypeScript,
preserving runtime behaviour and the on-the-wire encryption formats
exactly. Only module syntax and type annotations changed; the public
API is identical.

Build & packaging:
- Add tsconfig.json (strict) and build with tsup to dist/ as dual
  ESM (index.mjs) + CommonJS (index.js) with bundled .d.ts.
- Point package "main"/"module"/"types"/"exports" at dist and ship
  only dist; drop the tsc-based generate-types step.
- keepNames so error `type`/constructor names are preserved.
- require('@evervault/sdk') still returns the EvervaultClient class;
  `import Evervault from '@evervault/sdk'` works for ESM consumers.

Types & internals:
- Fold the hand-written types.d.ts / domainTargets.d.ts into source
  types; monkey-patched Node core modules use default imports so the
  mutable module.exports is patched (works in both CJS and ESM).

Tests & CI:
- Run the existing mocha suite against the TS source via tsx and
  replace rewire (incompatible with compiled TS) with proxyquire /
  shared-singleton config mutation. 204 passing, unchanged from the
  JS baseline (the 5 proxy.test.js failures are pre-existing and
  environmental).
- Add typecheck + build steps to CI; bump CodeQL to v3 with the
  javascript-typescript language.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011HwoTRLdV2YMub47j88mv5
@changeset-bot

changeset-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 169f0ed

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@evervault/sdk Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@socket-security

socket-security Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedproxyquire@​2.1.310010010075100
Added@​types/​async-retry@​1.4.91001009180100
Added@​types/​node@​22.20.11001008195100
Addedtsx@​4.23.01001008294100
Addedtsup@​8.5.1981009583100

View full report

claude and others added 5 commits July 9, 2026 11:16
…t config in sdk tests

CI ran the suite against the real network and surfaced two test-infra
regressions from the TS migration that the local sandbox hid:

- EvervaultClient's constructor called `_shouldOverloadHttpModule`, whose
  else-branch unconditionally ran `https.request = originalRequest`. On every
  non-relay client this reset the global `https.request`, removing nock's
  interception (nock doesn't re-patch once "active"), which cascaded failures
  across client/http test files. Guard the restore so it only runs when this
  process actually overloaded `https.request` for Relay. This is also more
  correct: a plain client no longer disables another client's outbound Relay.

- sdk.test.js pointed the client at its mock server by mutating the config
  singleton. Under tsx the module-cache timing made that unreliable, so the
  client hit the real API. Inject the mutated config into the client with
  proxyquire (`{ './config': config }`), mirroring the old rewire `__set__`.

Local suite unchanged (204 passing; the 5 proxy.test.js failures are
environmental to this sandbox). Verified the guard preserves nock's patch and
that config injection reaches the client.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011HwoTRLdV2YMub47j88mv5
The nock-based tests passed locally but failed in CI only. Running the
suite through tsx (on-the-fly TS transpilation with a custom module
loader) interacted with nock/axios HTTP interception differently on the
CI runners, so requests bypassed nock and hit the network.

Compile lib/*.ts to CJS with `tsc -p tsconfig.build.json` and run mocha
against the emitted JS under plain Node — the same execution model the
JavaScript suite used before the TypeScript migration. The compiled
lib/*.js are build artifacts (git- and prettier-ignored); tsup still
builds the published dual-format bundle from the .ts sources, and
`tsc --noEmit` still type-checks.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011HwoTRLdV2YMub47j88mv5
Mocha concatenates the `spec` from `.mocharc.json` with any CLI
positional file arguments rather than letting the CLI override it. The
`test:e2e` script (run by the `e2e.yml` workflow) invokes
`mocha 'e2e/**/*.test.js'`, so once `.mocharc.json` declared
`spec: tests/**`, that job silently ran the entire unit suite alongside
the e2e tests. The e2e tests run first, call `enableOutboundRelay()`
which monkey-patches the global `https.request`, and that leaves nock
unable to intercept the unit tests — producing the CI-only failures.

Keep only `timeout` in `.mocharc.json` and pass the unit spec explicitly
on the CLI in `test` / `test:filter`, so each mocha invocation resolves
exactly one suite (matching the pre-TypeScript setup).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011HwoTRLdV2YMub47j88mv5
Give consumers precise types instead of `any` on the client surface:

- encrypt<T> returns EncryptedData<T>, preserving input shape (objects keep
  their keys with encrypted string leaves, Buffers stay Buffers, primitives
  become strings) and rejecting non-encryptable inputs
- decrypt<T>, run<T> (-> FunctionRunResult<T>), createRunToken (-> RunToken)
  and createClientSideDecryptToken (-> ClientSideToken) now carry real types
- hidden ECDH fields typed as Buffer / crypto.ECDH / NodeJS.Timeout

Internally, export a reusable HttpClient type and thread it through
attestationDoc/relayOutboundConfig/httpsHelper; type the PCR store, the
attestation helpers, and the key/token/relay response shapes.

Also make Http.getAppKey throw mapResponseCodeToError on non-2xx (mirroring
getCageKey) instead of returning undefined and crashing downstream.

Genuinely dynamic/vendored surfaces (crypto key material that is Buffer|string,
the agent-base subclass, the asn1js DER encoder, the https.request/tls
monkeypatch paths) are left untyped with rationale comments.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The two path-filtering outbound-relay tests each make two httpbin.org
requests; the 5s timeout was too tight for slow-but-reachable httpbin,
causing intermittent timeouts. Align with the .mocharc.json default (30s).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@matt-evervault
matt-evervault requested a review from lfarrel6 July 17, 2026 14:47
@matt-evervault
matt-evervault marked this pull request as ready for review July 17, 2026 14:47
@matt-evervault matt-evervault self-assigned this Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Automated review

Reviewed the full diff (2,256 additions / 1,187 deletions across 50 files — the TS conversion, tsup dual ESM/CJS build, and test conversions from rewire to proxyquire). CI is green. Findings below, most severe first.

Should-fix

1. httpsRequestOverloaded is a process-global flag shared across all client instances (lib/index.ts, _shouldOverloadHttpModule)

let httpsRequestOverloaded = false;
...
} else if (httpsRequestOverloaded) {
  (https as any).request = originalRequest;
  httpsRequestOverloaded = false;
}

The JS baseline unconditionally reset https.request in the else branch; this version gates the reset on a module-level singleton instead of being per-instance. Concretely: construct client1 = new EvervaultClient(app1, key1, { enableOutboundRelay: true }) (patches https.request, sets the flag), then construct client2 = new EvervaultClient(app2, key2) with no relay options — client2's constructor sees the global flag is true and resets https.request back to the original, silently killing client1's Relay-based outbound decryption with no error. This is a real behavior change in a security-relevant path that isn't mentioned in the changeset (which describes the conversion as behavior-preserving). Might be worth scoping this per-instance (WeakSet/refcount) rather than a single boolean, or at least calling it out explicitly.

2. exports map may not correctly serve types to ESM consumers (package.json / tsup.config.ts / lib/index.ts)

"exports": {
  ".": {
    "types": "./dist/index.d.ts",
    "import": "./dist/index.mjs",
    "require": "./dist/index.js"
  }
}

lib/index.ts uses TS's CJS-only export =, and tsup with format: ['cjs','esm'], dts: true typically also emits index.d.mts for the ESM build. Here types is a single flat key rather than nested inside each of import/require, so under moduleResolution: "bundler"/"node16"/"nodenext" an ESM consumer's import Evervault from '@evervault/sdk' may resolve types through a mismatched declaration (or never see a generated .d.mts at all). Worth verifying with a small consumer project under node16/bundler resolution — if there's a mismatch, the standard fix is nesting types under each of import/require pointing at format-specific .d.ts/.d.mts files.

Nits (undisclosed but likely-intentional behavior drift)

3. lib/core/http.ts getAppKey now throws on non-2xx responses (throw errors.mapResponseCodeToError(response)), where the JS baseline fell off the end of the function and resolved undefined. Real improvement, but contradicts the "runtime behaviour unchanged" claim — worth a changeset line.

4. lib/core/http.ts getRelayOutboundConfig pollInterval parsing changed from isNaN(pollIntervalHeaderValue) ? null : parseFloat(...) (strict Number() coercion first) to always running lenient parseFloat first. For a malformed value like "10abc": old → null, new → 10. Low real-world impact (server-controlled header) but is a behavior drift from the refactor.

5. Type-safety is uneven — the public surface (encrypt<T>, EncryptedData<T>) is well-typed, but the highest-risk internal code is any-typed throughout: lib/core/crypto.ts (ecdhTeamKey, ecdhPublicKey, _traverseObject/_encryptString/_encryptBytes), lib/utils/proxyAgent.ts (fully any), lib/curves/base.ts (ASN1, curveParams). Understandable given awkward upstream typings (asn1js, agent-base), but worth noting the TS conversion buys the least safety exactly where mistakes would be most costly (on-the-wire crypto formatting).

6. SdkOptions.encryptionMode?: boolean is now a typed public field, and validationHelper.validateApiKey supports an early-return bypass on options.encryptionMode === true, but no visible call site in lib/index.ts forwards an options object to validateApiKey (all calls pass only appId/apiKey), and there's no visible assignment of this.encryptionMode from constructor options. If this is genuinely new, it looks unwired; if carried over from the stacked base branch, it's now just exposed as public API — worth confirming that's intentional.

Checked, no issues found

  • Monkey-patch live-binding correctness across CJS/ESM builds: all mutation sites (lib/index.ts, lib/utils/httpsHelper.ts, lib/utils/attest.ts) use default imports (import https from 'https'), which correctly point at the live shared module object under both esbuild's CJS output and native ESM — no bug from the dual-build target.
  • error.type preservation: EvervaultError sets this.type = this.constructor.name after super(), unaffected by useDefineForClassFields field-init ordering.
  • rewireproxyquire conversion: checked all 7 converted test files; stubbing and .default access correctly matches tsc/tsup's export default emission, no silent no-ops. One purely stylistic inconsistency in tests/sdk.test.js (4/5 config re-injections go through proxyquire, one mutates the shared singleton directly) — functionally equivalent, not a bug.

Automated review — flag anything above that doesn't hold up.


Generated by Claude Code

Comment thread lib/core/http.ts
});
if (response.status >= 200 && response.status < 300) {
const pollIntervalHeaderValue = response.headers['x-poll-interval'];
const pollInterval = parseFloat(String(pollIntervalHeaderValue));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is potentially changing behavior on corner cases

Comment thread lib/core/repeatedTimer.ts
console.error(`EVERVAULT :: An error occurred while polling (${e})`);
}
}, interval * 1000);
}, (interval as number) * 1000);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to check if this is a number instead of coercing it to a number

Comment thread lib/curves/base.ts
'hex',
'uncompressed'
);
) as string;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use toString instead

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants