Closed
Conversation
Domain types (ISecretProvider, MapFileConfig, EnvilderOptions, ParsedMapFile, SecretProviderType), application layer (Envilder facade, EnvilderClient, MapFileParser, validateSecrets), and infrastructure (AwsSsmSecretProvider, AzureKeyVaultSecretProvider, createSecretProvider factory). Async-first API with fluent builder pattern. Supports AWS SSM Parameter Store and Azure Key Vault. Published as @envilder/sdk.
46 tests covering MapFileParser (6), EnvilderClient (5), SecretValidation (5), AwsSsmSecretProvider (5), AzureKeyVaultSecretProvider (5), SecretProviderFactory (7), and Envilder facade (13) with environment routing and fluent builder. Vitest with vi.fn() mocks at port boundaries.
- Add TypeScript SDK to landing page (available status with code example and npm package link) - Add #sdk-typescript docs section with install, quick start, resolve, fluent builder, env routing, and validation examples - Add i18n translations for TypeScript SDK (en, ca, es) - Add changelog infrastructure (sdk-typescript.md, astro config, env.d.ts type declaration) - Move TypeScript SDK from Up Next to Shipped in ROADMAP.md - Add TypeScript SDK section to copilot-instructions.md
Contributor
|
Caution Review failedPull request was closed or merged during review 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:
WalkthroughThis pull request introduces a complete Node.js SDK implementation for Envilder that loads secrets from AWS SSM or Azure Key Vault into Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant Envilder as Envilder
participant Parser as MapFileParser
participant Factory as ProviderFactory
participant Provider as ISecretProvider<br/>(AWS/Azure)
participant Client as EnvilderClient
App->>Envilder: load(filePath)
Envilder->>Envilder: readFile(filePath)
Envilder->>Parser: parse(json)
Parser-->>Envilder: {config, mappings}
Envilder->>Factory: createSecretProvider(config)
Factory->>Provider: new AwsSsmSecretProvider()<br/>or new AzureKeyVaultSecretProvider()
Provider-->>Factory: provider instance
Factory-->>Envilder: provider
Envilder->>Client: new EnvilderClient(provider)
Client-->>Envilder: client
Envilder->>Client: resolveSecrets(mappings)
Client->>Provider: getSecrets(secretPaths)
Provider-->>Client: Map<name, value>
Client-->>Envilder: resolved secrets Map
Envilder->>Envilder: injectIntoEnvironment(secrets)
Envilder-->>App: Promise<Map>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Set root to repo root and use relative include patterns so v8 coverage provider can instrument SDK source files. Also fix the resolve alias path (was 2 levels up, needed 3).
Replace !x || !x.trim() with !x?.trim() in 7 locations across facade, validation, providers, and factory modules.
Split all // Act & Assert combined comments into separate // Act and // Assert phases. Rewrite try/catch test to use expect().toThrow(). Add missing Should_ResolveFromMappedFile_When_EnvMappingProvided test for resolveFile with environment routing.
Add test-typescript-sdk job that runs vitest with v8 coverage and generates lcov report. Wire into publish-report needs array, failure check, and coverage-config.json gate (80% line threshold).
Add tests-typescript-sdk.yml: biome check, build, vitest run with junit reporter, and dorny/test-reporter for PR annotations. Add publish-npm-sdk.yml: version bump detection against npm registry, lint + build + test gate, pnpm publish, git tag, GitHub Release.
Replace NPM_TOKEN secret with npm publish --provenance to match the CLI publish workflow. OIDC is already configured via id-token permission and registry-url in setup-node.
Change pre-push biome-format from read-only 'pnpm format' to 'pnpm format:write' so formatting issues are fixed automatically instead of blocking the push.
The format:write in pre-push creates an infinite loop: it modifies files without committing, so the next push triggers the same fix. Pre-push must be read-only. Auto-fix belongs in pre-commit only.
vitest root is set to repo root in vitest.config.ts, so relative outputFile paths resolve from there. Use github.workspace to write test-results.xml at the repo root where dorny/test-reporter can find it.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new independent TypeScript runtime SDK (@envilder/sdk) to load secrets into Node.js processes from AWS SSM / Azure Key Vault, plus a dedicated unit-test workspace and supporting docs/website/CI plumbing.
Changes:
- Introduces the TypeScript SDK (domain/application/infrastructure layers) and exports its public API.
- Adds a separate Vitest-based test workspace for the SDK and wires SDK coverage into the coverage dashboard workflow.
- Updates website/docs/i18n/roadmap/changelog entries to mark the TypeScript SDK as shipped and documented.
Reviewed changes
Copilot reviewed 47 out of 49 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/sdks/typescript/vitest.config.ts | Vitest configuration for the SDK test workspace (root/coverage/alias). |
| tests/sdks/typescript/tsconfig.json | TypeScript config for the SDK tests workspace. |
| tests/sdks/typescript/package.json | Test workspace package manifest (Vitest + TS dev deps). |
| tests/sdks/typescript/infrastructure/secret-provider-factory.test.ts | Unit tests for provider factory behavior and option/config precedence. |
| tests/sdks/typescript/infrastructure/azure/azure-key-vault-secret-provider.test.ts | Unit tests for Azure Key Vault provider behavior. |
| tests/sdks/typescript/infrastructure/aws/aws-ssm-secret-provider.test.ts | Unit tests for AWS SSM provider behavior and batching. |
| tests/sdks/typescript/application/secret-validation.test.ts | Unit tests for opt-in secret validation. |
| tests/sdks/typescript/application/map-file-parser.test.ts | Unit tests for map-file parsing ($config + mappings). |
| tests/sdks/typescript/application/envilder.test.ts | Unit tests for the public Envilder facade (file/env routing + fluent API). |
| tests/sdks/typescript/application/envilder-client.test.ts | Unit tests for core EnvilderClient resolution + injection. |
| tests/sdks/typescript/.gitkeep | Removes placeholder now that the folder contains real tests. |
| src/website/src/pages/es/changelog.astro | Adds TypeScript SDK changelog category to Spanish page. |
| src/website/src/pages/changelog.astro | Adds TypeScript SDK changelog category to default page. |
| src/website/src/pages/ca/changelog.astro | Adds TypeScript SDK changelog category to Catalan page. |
| src/website/src/i18n/types.ts | Adds i18n keys for TypeScript SDK docs + changelog labels. |
| src/website/src/i18n/es.ts | Spanish translations for new TypeScript SDK sections and status. |
| src/website/src/i18n/en.ts | English translations for new TypeScript SDK sections and status. |
| src/website/src/i18n/ca.ts | Catalan translations for new TypeScript SDK sections and status. |
| src/website/src/env.d.ts | Declares the new injected changelog constant for TypeScript SDK. |
| src/website/src/components/Sdks.astro | Marks TypeScript SDK as available and adds install/docs/code snippet. |
| src/website/src/components/DocsContent.astro | Adds full TypeScript SDK docs section + sidebar navigation entry. |
| src/website/astro.config.mjs | Loads and injects the new TypeScript SDK changelog content. |
| src/sdks/typescript/tsconfig.json | TypeScript SDK build config (Node16 ESM output + declarations). |
| src/sdks/typescript/src/infrastructure/secret-provider-factory.ts | Factory creating AWS/Azure providers and enforcing cross-provider config rules. |
| src/sdks/typescript/src/infrastructure/azure/azure-key-vault-secret-provider.ts | Azure Key Vault implementation of ISecretProvider. |
| src/sdks/typescript/src/infrastructure/aws/aws-ssm-secret-provider.ts | AWS SSM implementation of ISecretProvider with batching. |
| src/sdks/typescript/src/index.ts | Barrel exports for the SDK public API. |
| src/sdks/typescript/src/domain/secret-provider-type.ts | Provider enum (aws/azure). |
| src/sdks/typescript/src/domain/ports/secret-provider.ts | ISecretProvider interface contract for providers. |
| src/sdks/typescript/src/domain/parsed-map-file.ts | Parsed map-file type (config + mappings). |
| src/sdks/typescript/src/domain/map-file-config.ts | $config schema type for provider configuration. |
| src/sdks/typescript/src/domain/envilder-options.ts | Override options type for fluent/builder usage. |
| src/sdks/typescript/src/application/secret-validation.ts | Opt-in secret validation and error type. |
| src/sdks/typescript/src/application/map-file-parser.ts | JSON map-file parsing into typed config + mappings. |
| src/sdks/typescript/src/application/envilder.ts | Public facade (load/resolveFile/fromMapFile + env routing + fluent overrides). |
| src/sdks/typescript/src/application/envilder-client.ts | Core resolver + environment injection helper. |
| src/sdks/typescript/package.json | NPM package manifest for @envilder/sdk. |
| src/sdks/typescript/.gitkeep | Removes placeholder now that the SDK has real source. |
| pnpm-workspace.yaml | Adds SDK + test workspace packages to the pnpm workspace. |
| pnpm-lock.yaml | Adds dependency graph entries for the new workspaces and SDK deps. |
| package.json | Adjusts root Biome formatting scripts and lint behavior. |
| lefthook.yml | Expands pre-commit Biome coverage and runs check+format on staged files. |
| docs/changelogs/sdk-typescript.md | Adds the TypeScript SDK changelog file. |
| ROADMAP.md | Marks TypeScript SDK as shipped and removes it from “Up Next”. |
| .github/workflows/tests-typescript-sdk.yml | Adds a dedicated CI workflow for SDK formatting/build/tests. |
| .github/workflows/publish-npm-sdk.yml | Adds an NPM publish workflow for the SDK with version bump detection. |
| .github/workflows/coverage-report/coverage-config.json | Adds coverage threshold config for the TypeScript SDK stack. |
| .github/workflows/coverage-report.yml | Adds TypeScript SDK coverage job and includes it in aggregated dashboard. |
| .github/copilot-instructions.md | Documents the TypeScript SDK architecture, patterns, and test conventions. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
|
|
||
| for (const name of names) { | ||
| if (!name?.trim()) { | ||
| throw new Error('Secret name cannot be null or whitespace.'); |
Comment on lines
+4
to
+5
| const repoRoot = path.resolve(__dirname, '../../..'); | ||
|
|
| "format": "biome format", | ||
| "format:write": "biome format --write", | ||
| "lint": "secretlint \"**/*\" && biome check --write && tsc --noEmit", | ||
| "format": "biome check --write --unsafe && biome format --write", |
Comment on lines
+50
to
+54
|
|
||
| // Assert | ||
| expect(actual).toBeDefined(); | ||
| expect(actual.getSecret).toBeDefined(); | ||
| }); |
Comment on lines
+21
to
+29
| if (isAzure && profile) { | ||
| throw new Error( | ||
| 'AWS profile cannot be used with Azure Key Vault provider.', | ||
| ); | ||
| } | ||
|
|
||
| if (!isAzure && vaultUrl) { | ||
| throw new Error('Vault URL cannot be used with AWS SSM provider.'); | ||
| } |
| ); | ||
|
|
||
| for (const param of response.Parameters ?? []) { | ||
| if (param.Name && param.Value) { |
Comment on lines
+159
to
+166
| specifier: ^22.0.0 | ||
| version: 22.19.17 | ||
| rimraf: | ||
| specifier: ^6.0.0 | ||
| version: 6.1.3 | ||
| typescript: | ||
| specifier: ^5.7.0 | ||
| version: 5.9.3 |
Comment on lines
+14
to
+17
| "paths": { | ||
| "@envilder/sdk": ["../../src/sdks/typescript/src/index.js"], | ||
| "@envilder/sdk/*": ["../../src/sdks/typescript/src/*"] | ||
| } |
|
|
||
| for (const name of names) { | ||
| if (!name?.trim()) { | ||
| throw new Error('Secret name cannot be null or empty.'); |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/website/src/pages/ca/changelog.astro (1)
32-166:⚠️ Potential issue | 🔴 CriticalSame desktop-sidebar index drift as
es/changelog.astro.Inserting
sdk-typescriptat index 3 makes GHAparsed[4], but the desktop sidebar still hardcodesparsed[3]for the GHA button/version list (Lines 155, 158–166). Result: GHA shows TypeScript counts/links and there is no sidebar entry for TypeScript on desktop. Fix alongside the root cause flagged insrc/website/src/pages/es/changelog.astro(and likelysrc/website/src/pages/changelog.astro) — ideally by addressing the products byidinstead of positional index.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/website/src/pages/ca/changelog.astro` around lines 32 - 166, The desktop sidebar incorrectly indexes into parsed by position (e.g., using parsed[3] for GHA), causing misaligned product counts/links when products like 'sdk-typescript' are inserted; update the sidebar rendering logic to look up products by id instead of hardcoded array indices: use parsed.find(p => p.id === 'gha') (and similarly for 'cli', 'sdk-dotnet', 'sdk-python', etc.) to compute versions/counts and to map versions for the <ul class="version-list" data-versions="..."> and corresponding <button> elements (references: parsed, data-product attributes, data-versions values, product-select-mobile, and the GHA button/version-list sections) so the desktop sidebar always renders the correct product regardless of array ordering.src/website/src/pages/es/changelog.astro (1)
32-166:⚠️ Potential issue | 🔴 CriticalCritical: desktop sidebar indices are stale after inserting
sdk-typescript.The desktop sidebar hardcodes
parsed[3]for the GHA section (Lines 155, 158) under the assumption that the products array order is[cli, sdk-dotnet, sdk-python, gha]. Withsdk-typescriptinserted at index 3, GHA is now atparsed[4], so:
- The GHA button shows the TypeScript version count (Line 155).
- The GHA
<ul>lists TypeScript versions linked withdata-product="gha"(Lines 158–166), so version anchors go to the wrong product and break navigation.- There is no desktop sidebar button or version list for sdk-typescript at all — only the mobile selector renders it.
The same bug exists in
src/website/src/pages/ca/changelog.astroand likely insrc/website/src/pages/changelog.astro. Consider rendering the sidebar fromparsedbyid(lookup map) rather than positional indices, so future additions don't desynchronize.🛠️ Suggested approach (sketch)
+const byId = Object.fromEntries(parsed.map((p) => [p.id, p])); ... - <span class="nav-product-count">{parsed[3].versions.length}</span> + <span class="nav-product-count">{byId['gha'].versions.length}</span> ... - {parsed[3].versions.map((v) => ( + {byId['gha'].versions.map((v) => (And add a TypeScript button/list block between the Python and GHA blocks (or render the SDK group from
parsed.filter(p => p.group === 'sdks')).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/website/src/pages/es/changelog.astro` around lines 32 - 166, The desktop sidebar uses fixed positional indices into the parsed array (e.g., parsed[0], parsed[1], parsed[2], parsed[3]) which became stale after inserting sdk-typescript; update the sidebar rendering logic (the product-nav-desktop block that builds the buttons and <ul class="version-list">) to look up products by id or iterate parsed instead of using hardcoded indices—e.g., build a lookup like parsedById or loop parsed.filter(p => p.group === 'sdks') and render each product's button and versions using p.id, p.versions, and p.label so data-product attributes and href anchors (e.g., `#${p.id}-${v.id}`) stay correct and sdk-typescript is included.
🧹 Nitpick comments (3)
src/sdks/typescript/src/infrastructure/azure/azure-key-vault-secret-provider.ts (1)
27-29: Minor: error message inconsistency with AWS provider.The AWS provider throws
'Secret name cannot be null or whitespace.'while this throws'Secret name cannot be null or empty.'for the same!name?.trim()check. Aligning the wording avoids surprising consumers depending on which provider is selected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sdks/typescript/src/infrastructure/azure/azure-key-vault-secret-provider.ts` around lines 27 - 29, The Azure provider's validation in azure-key-vault-secret-provider.ts uses if (!name?.trim()) in the method that validates secret names and throws "Secret name cannot be null or empty."; update that Error message to match the AWS provider wording ("Secret name cannot be null or whitespace.") so both providers are consistent—locate the check in the AzureKeyVaultSecretProvider (or the method performing name validation) and replace the thrown message accordingly.src/sdks/typescript/src/infrastructure/aws/aws-ssm-secret-provider.ts (1)
37-51: Optional: parallelize batches for many secrets.Batches are fetched sequentially via
awaitin the loop. For map files with >10 secrets this serializes round-trips to SSM. Since the Azure provider usesPromise.allfor concurrency andISecretProviderdoes not prescribe ordering, you could collect each batch's promise andawait Promise.all(...)to halve latency on larger maps.♻️ Proposed parallelization
- for (let i = 0; i < names.length; i += SSM_BATCH_SIZE) { - const batch = names.slice(i, i + SSM_BATCH_SIZE); - const response = await this.ssmClient.send( - new GetParametersCommand({ - Names: batch, - WithDecryption: true, - }), - ); - - for (const param of response.Parameters ?? []) { - if (param.Name && param.Value) { - result.set(param.Name, param.Value); - } - } - } + const batches: Promise<Awaited<ReturnType<typeof this.ssmClient.send>>>[] = []; + for (let i = 0; i < names.length; i += SSM_BATCH_SIZE) { + const batch = names.slice(i, i + SSM_BATCH_SIZE); + batches.push( + this.ssmClient.send( + new GetParametersCommand({ + Names: batch, + WithDecryption: true, + }), + ), + ); + } + const responses = await Promise.all(batches); + for (const response of responses) { + for (const param of response.Parameters ?? []) { + if (param.Name && param.Value) { + result.set(param.Name, param.Value); + } + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sdks/typescript/src/infrastructure/aws/aws-ssm-secret-provider.ts` around lines 37 - 51, The current loop fetches SSM batches serially; change it to fire all batch requests concurrently by collecting promises from this.ssmClient.send(new GetParametersCommand({ Names: batch, WithDecryption: true })) for each batch (using SSM_BATCH_SIZE and names as before), then await Promise.all on that array; after Promise.all resolves, iterate each response.Parameters (as you currently do) to populate result.set(param.Name, param.Value). Keep use of SSM_BATCH_SIZE, GetParametersCommand, ssmClient.send and result intact and ensure you still handle response.Parameters ?? [] for each resolved response.src/sdks/typescript/src/application/envilder.ts (1)
56-93: Staticload/resolveFilecannot apply provider overrides.The static
load(...)andresolveFile(...)paths constructnew Envilder(source)directly, bypassingwithProvider/withVaultUrl/withProfile. This is by design — overrides require the fluent builder — but worth noting in JSDoc so callers understand they must useEnvilder.fromMapFile(...)(and re-implement env routing) when they need overrides. Otherwise, on Azure-configured maps, users could be surprised thatload('production', envMapping)works only when the map file's$configalready declares everything needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sdks/typescript/src/application/envilder.ts` around lines 56 - 93, The static methods load(...) and resolveFile(...) construct new Envilder(source) directly and therefore bypass any provider overrides set via the fluent builder (e.g., withProvider, withVaultUrl, withProfile), so update the JSDoc for both Envilder.load and Envilder.resolveFile to explicitly state that provider/profile/vault overrides are not applied when using these static convenience paths and that callers who need overrides should use the fluent builder (e.g., Envilder.fromMapFile(...).withProvider(...).withVaultUrl(...).withProfile(...)) or re-run env routing via resolveEnvSource before calling the builder; reference the symbols load, resolveFile, Envilder, withProvider, withVaultUrl, withProfile, fromMapFile, and resolveEnvSource in the comment so consumers are guided to the correct alternative.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/sdks/typescript/src/infrastructure/secret-provider-factory.ts`:
- Around line 12-36: The createSecretProvider function currently treats any
provider value other than SecretProviderType.Azure as AWS; add explicit
validation: if options?.provider or config.provider is present but not equal to
a known SecretProviderType value, throw a clear error before branching to
isAzure—e.g., validate provider against SecretProviderType enum, then proceed to
use createAzureProvider(vaultUrl) when provider === SecretProviderType.Azure and
createAwsProvider(profile) when provider === SecretProviderType.AWS; this
ensures unknown strings (like "gcp" or "AWS" typo) fail fast with a descriptive
message referencing createSecretProvider and SecretProviderType.
In `@src/website/src/pages/changelog.astro`:
- Around line 32-37: The desktop sidebar is using hardcoded indices on parsed
causing the GHA block to read parsed[3] and miss the newly inserted
sdk-typescript; update the hand-rolled desktop sidebar to either (A) insert a
dedicated TypeScript SDK block that reads the sdk-typescript entry from parsed
(id 'sdk-typescript') and change the GHA block to use parsed[4] (id 'gha'), or
(B — preferred) replace the index lookups with lookups by id (e.g.,
parsed.find(p => p.id === 'sdk-typescript') and parsed.find(p => p.id ===
'gha')) so the GHA button/version list and the new TypeScript sidebar entry
always reference the correct product regardless of array order.
- Around line 32-37: The locale changelog pages use hardcoded
parsed[0]..parsed[3] references which breaks if the categories order changes;
update each locale changelog.astro to locate categories dynamically instead of
using parsed[0]/parsed[1]/parsed[2]/parsed[3] — e.g., replace direct indexed
access of parsed with a lookup like parsed.find(p => p.id === 'sdk-typescript'
|| p.group === 'sdks') (or find by label using t.changelogPage keys) and fall
back to __CHANGELOG_SDK_TYPESCRIPT__ || fallback when the lookup returns
undefined; ensure you update all occurrences of parsed[0..3] in the locale files
so the sidebar renders correctly regardless of array order.
---
Outside diff comments:
In `@src/website/src/pages/ca/changelog.astro`:
- Around line 32-166: The desktop sidebar incorrectly indexes into parsed by
position (e.g., using parsed[3] for GHA), causing misaligned product
counts/links when products like 'sdk-typescript' are inserted; update the
sidebar rendering logic to look up products by id instead of hardcoded array
indices: use parsed.find(p => p.id === 'gha') (and similarly for 'cli',
'sdk-dotnet', 'sdk-python', etc.) to compute versions/counts and to map versions
for the <ul class="version-list" data-versions="..."> and corresponding <button>
elements (references: parsed, data-product attributes, data-versions values,
product-select-mobile, and the GHA button/version-list sections) so the desktop
sidebar always renders the correct product regardless of array ordering.
In `@src/website/src/pages/es/changelog.astro`:
- Around line 32-166: The desktop sidebar uses fixed positional indices into the
parsed array (e.g., parsed[0], parsed[1], parsed[2], parsed[3]) which became
stale after inserting sdk-typescript; update the sidebar rendering logic (the
product-nav-desktop block that builds the buttons and <ul class="version-list">)
to look up products by id or iterate parsed instead of using hardcoded
indices—e.g., build a lookup like parsedById or loop parsed.filter(p => p.group
=== 'sdks') and render each product's button and versions using p.id,
p.versions, and p.label so data-product attributes and href anchors (e.g.,
`#${p.id}-${v.id}`) stay correct and sdk-typescript is included.
---
Nitpick comments:
In `@src/sdks/typescript/src/application/envilder.ts`:
- Around line 56-93: The static methods load(...) and resolveFile(...) construct
new Envilder(source) directly and therefore bypass any provider overrides set
via the fluent builder (e.g., withProvider, withVaultUrl, withProfile), so
update the JSDoc for both Envilder.load and Envilder.resolveFile to explicitly
state that provider/profile/vault overrides are not applied when using these
static convenience paths and that callers who need overrides should use the
fluent builder (e.g.,
Envilder.fromMapFile(...).withProvider(...).withVaultUrl(...).withProfile(...))
or re-run env routing via resolveEnvSource before calling the builder; reference
the symbols load, resolveFile, Envilder, withProvider, withVaultUrl,
withProfile, fromMapFile, and resolveEnvSource in the comment so consumers are
guided to the correct alternative.
In `@src/sdks/typescript/src/infrastructure/aws/aws-ssm-secret-provider.ts`:
- Around line 37-51: The current loop fetches SSM batches serially; change it to
fire all batch requests concurrently by collecting promises from
this.ssmClient.send(new GetParametersCommand({ Names: batch, WithDecryption:
true })) for each batch (using SSM_BATCH_SIZE and names as before), then await
Promise.all on that array; after Promise.all resolves, iterate each
response.Parameters (as you currently do) to populate result.set(param.Name,
param.Value). Keep use of SSM_BATCH_SIZE, GetParametersCommand, ssmClient.send
and result intact and ensure you still handle response.Parameters ?? [] for each
resolved response.
In
`@src/sdks/typescript/src/infrastructure/azure/azure-key-vault-secret-provider.ts`:
- Around line 27-29: The Azure provider's validation in
azure-key-vault-secret-provider.ts uses if (!name?.trim()) in the method that
validates secret names and throws "Secret name cannot be null or empty."; update
that Error message to match the AWS provider wording ("Secret name cannot be
null or whitespace.") so both providers are consistent—locate the check in the
AzureKeyVaultSecretProvider (or the method performing name validation) and
replace the thrown message accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f7fa5df1-b647-4b69-8428-f561bde7628b
⛔ Files ignored due to path filters (7)
.github/copilot-instructions.mdis excluded by none and included by none.github/workflows/coverage-report.ymlis excluded by none and included by none.github/workflows/coverage-report/coverage-config.jsonis excluded by none and included by none.github/workflows/publish-npm-sdk.ymlis excluded by none and included by none.github/workflows/tests-typescript-sdk.ymlis excluded by none and included by nonelefthook.ymlis excluded by none and included by nonepackage.jsonis excluded by none and included by none
📒 Files selected for processing (26)
src/sdks/typescript/package.jsonsrc/sdks/typescript/src/application/envilder-client.tssrc/sdks/typescript/src/application/envilder.tssrc/sdks/typescript/src/application/map-file-parser.tssrc/sdks/typescript/src/application/secret-validation.tssrc/sdks/typescript/src/domain/ports/secret-provider.tssrc/sdks/typescript/src/infrastructure/aws/aws-ssm-secret-provider.tssrc/sdks/typescript/src/infrastructure/azure/azure-key-vault-secret-provider.tssrc/sdks/typescript/src/infrastructure/secret-provider-factory.tssrc/website/src/components/DocsContent.astrosrc/website/src/components/Sdks.astrosrc/website/src/i18n/ca.tssrc/website/src/i18n/en.tssrc/website/src/i18n/es.tssrc/website/src/i18n/types.tssrc/website/src/pages/ca/changelog.astrosrc/website/src/pages/changelog.astrosrc/website/src/pages/es/changelog.astrotests/sdks/typescript/application/envilder-client.test.tstests/sdks/typescript/application/envilder.test.tstests/sdks/typescript/application/secret-validation.test.tstests/sdks/typescript/infrastructure/aws/aws-ssm-secret-provider.test.tstests/sdks/typescript/infrastructure/azure/azure-key-vault-secret-provider.test.tstests/sdks/typescript/infrastructure/secret-provider-factory.test.tstests/sdks/typescript/package.jsontests/sdks/typescript/vitest.config.ts
✅ Files skipped from review due to trivial changes (4)
- tests/sdks/typescript/package.json
- tests/sdks/typescript/application/secret-validation.test.ts
- src/sdks/typescript/package.json
- src/sdks/typescript/src/application/envilder-client.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- src/sdks/typescript/src/domain/ports/secret-provider.ts
- tests/sdks/typescript/vitest.config.ts
- tests/sdks/typescript/infrastructure/aws/aws-ssm-secret-provider.test.ts
- tests/sdks/typescript/infrastructure/azure/azure-key-vault-secret-provider.test.ts
- src/website/src/i18n/types.ts
- src/sdks/typescript/src/application/map-file-parser.ts
- src/sdks/typescript/src/application/secret-validation.ts
- tests/sdks/typescript/application/envilder-client.test.ts
- src/website/src/i18n/ca.ts
- tests/sdks/typescript/infrastructure/secret-provider-factory.test.ts
Comment on lines
+12
to
+36
| export function createSecretProvider( | ||
| config: MapFileConfig, | ||
| options?: EnvilderOptions, | ||
| ): ISecretProvider { | ||
| const provider = options?.provider ?? config.provider; | ||
| const profile = options?.profile ?? config.profile; | ||
| const vaultUrl = options?.vaultUrl ?? config.vaultUrl; | ||
| const isAzure = provider === SecretProviderType.Azure; | ||
|
|
||
| if (isAzure && profile) { | ||
| throw new Error( | ||
| 'AWS profile cannot be used with Azure Key Vault provider.', | ||
| ); | ||
| } | ||
|
|
||
| if (!isAzure && vaultUrl) { | ||
| throw new Error('Vault URL cannot be used with AWS SSM provider.'); | ||
| } | ||
|
|
||
| if (isAzure) { | ||
| return createAzureProvider(vaultUrl); | ||
| } | ||
|
|
||
| return createAwsProvider(profile); | ||
| } |
Contributor
There was a problem hiding this comment.
Unknown provider values silently fall through to AWS.
isAzure is a strict equality check, so any provider value other than SecretProviderType.Azure (including a malformed string from a hand-written $config, e.g. "gcp" or a typo like "AWS") is silently treated as AWS. Consider failing fast with a clear error when provider is set but not a known SecretProviderType value, so users get a useful diagnostic instead of unexpected AWS calls.
🛡️ Proposed validation
const provider = options?.provider ?? config.provider;
const profile = options?.profile ?? config.profile;
const vaultUrl = options?.vaultUrl ?? config.vaultUrl;
+ if (
+ provider !== undefined &&
+ provider !== SecretProviderType.Aws &&
+ provider !== SecretProviderType.Azure
+ ) {
+ throw new Error(`Unknown secret provider: '${provider}'.`);
+ }
const isAzure = provider === SecretProviderType.Azure;📝 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 createSecretProvider( | |
| config: MapFileConfig, | |
| options?: EnvilderOptions, | |
| ): ISecretProvider { | |
| const provider = options?.provider ?? config.provider; | |
| const profile = options?.profile ?? config.profile; | |
| const vaultUrl = options?.vaultUrl ?? config.vaultUrl; | |
| const isAzure = provider === SecretProviderType.Azure; | |
| if (isAzure && profile) { | |
| throw new Error( | |
| 'AWS profile cannot be used with Azure Key Vault provider.', | |
| ); | |
| } | |
| if (!isAzure && vaultUrl) { | |
| throw new Error('Vault URL cannot be used with AWS SSM provider.'); | |
| } | |
| if (isAzure) { | |
| return createAzureProvider(vaultUrl); | |
| } | |
| return createAwsProvider(profile); | |
| } | |
| export function createSecretProvider( | |
| config: MapFileConfig, | |
| options?: EnvilderOptions, | |
| ): ISecretProvider { | |
| const provider = options?.provider ?? config.provider; | |
| const profile = options?.profile ?? config.profile; | |
| const vaultUrl = options?.vaultUrl ?? config.vaultUrl; | |
| if ( | |
| provider !== undefined && | |
| provider !== SecretProviderType.Aws && | |
| provider !== SecretProviderType.Azure | |
| ) { | |
| throw new Error(`Unknown secret provider: '${provider}'.`); | |
| } | |
| const isAzure = provider === SecretProviderType.Azure; | |
| if (isAzure && profile) { | |
| throw new Error( | |
| 'AWS profile cannot be used with Azure Key Vault provider.', | |
| ); | |
| } | |
| if (!isAzure && vaultUrl) { | |
| throw new Error('Vault URL cannot be used with AWS SSM provider.'); | |
| } | |
| if (isAzure) { | |
| return createAzureProvider(vaultUrl); | |
| } | |
| return createAwsProvider(profile); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/sdks/typescript/src/infrastructure/secret-provider-factory.ts` around
lines 12 - 36, The createSecretProvider function currently treats any provider
value other than SecretProviderType.Azure as AWS; add explicit validation: if
options?.provider or config.provider is present but not equal to a known
SecretProviderType value, throw a clear error before branching to isAzure—e.g.,
validate provider against SecretProviderType enum, then proceed to use
createAzureProvider(vaultUrl) when provider === SecretProviderType.Azure and
createAwsProvider(profile) when provider === SecretProviderType.AWS; this
ensures unknown strings (like "gcp" or "AWS" typo) fail fast with a descriptive
message referencing createSecretProvider and SecretProviderType.
- Add LocalStack container wrapper for AWS SSM acceptance tests - Add Lowkey Vault container wrapper for Azure Key Vault acceptance tests - Add 2 AWS SSM acceptance tests (resolve + missing parameter) - Add 2 Azure Key Vault acceptance tests (resolve + missing secret) - Add 2 unit tests for MapFileParser (invalid root, unknown provider) - Add 2 unit tests for Envilder facade (null env mapping, missing env) - Fix toThrowError deprecation warnings (use toThrow) - Update CI workflows with OIDC credentials and Docker env for TestContainers - Add ADR-0001 documenting SDK acceptance test infrastructure decisions - Add sdk-acceptance-testing Copilot skill for cross-SDK test patterns
macalbert
had a problem deploying
to
github-pages
GitHub Actions
Failure
Co-authored-by: Copilot <copilot@github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
tests/sdks/typescript/application/envilder.test.ts (3)
99-110: Inconsistent assertion coverage forprocess.env.Other resolve-only tests (e.g., line 95-96) assert both
DB_URLandAPI_KEYare undefined, but this test only checksDB_URL. For symmetry and to guard against partial-injection regressions, assertAPI_KEYis also undefined.♻️ Proposed addition
expect(actual.size).toBe(2); expect(process.env.DB_URL).toBeUndefined(); + expect(process.env.API_KEY).toBeUndefined(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/sdks/typescript/application/envilder.test.ts` around lines 99 - 110, Update the test Should_ResolveFromMappedFile_When_EnvMappingProvided to assert both environment variables are not injected: after calling Envilder.resolveFile('production', ...) and verifying mockReadFile was called, add an assertion that process.env.API_KEY is undefined in addition to the existing process.env.DB_URL check to mirror other resolve-only tests and prevent partial-injection regressions; reference Envilder.resolveFile and mockReadFile in your change.
51-56: RedundantrestoreAllMocksalongsideclearAllMocks.
vi.restoreAllMocks()is intended for spies created viavi.spyOn()and restores their original implementations. The module mocks here are defined viavi.mock()factories, soclearAllMocks()(which resets call history) is sufficient. CallingrestoreAllMocks()onvi.fn(impl)instances likemockProvider.getSecretswill revert their implementation — the nextbeforeEachresetsmockReadFileandmockCreateProvider, butmockProvider.getSecrets's impl is not re-set, so subsequent tests rely on Vitest preservingvi.fn(impl)behavior across restore. Simpler and more predictable to droprestoreAllMocks().♻️ Proposed cleanup
afterEach(() => { vi.clearAllMocks(); - vi.restoreAllMocks(); delete process.env.DB_URL; delete process.env.API_KEY; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/sdks/typescript/application/envilder.test.ts` around lines 51 - 56, Remove the redundant vi.restoreAllMocks() call from the afterEach block in the test file: keep vi.clearAllMocks() and the environment cleanup but delete vi.restoreAllMocks() because mocks are created via vi.mock() and individual vi.fn() implementations such as mockProvider.getSecrets (and other mocks like mockReadFile and mockCreateProvider) should not have their implementations reverted by restoreAllMocks; this ensures each beforeEach can reliably set mock implementations without unexpected resets.
87-187: Routing tests are split across twodescribeblocks with duplicate names.
resolveFile(lines 99-134) andenvironment routing(lines 137-187) both cover env-mapping behavior, with two identically named tests (Should_ReturnEmpty_When_EnvMapsToNull,Should_ReturnEmpty_When_EnvNotInMapping). The split makes it harder to see routing coverage at a glance, andenvironment routingonly exercisesloadwhile routing forresolveFilelives in the other block.Consider consolidating routing tests under a single
describe('environment routing')parameterized overload/resolveFile, or moving theresolveFileenv-mapping cases into the routing block with distinct names (e.g.,..._When_ResolveFileCalledvs..._When_LoadCalled).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/sdks/typescript/application/envilder.test.ts` around lines 87 - 187, Tests for env-mapping behavior are split between describe('resolveFile') and describe('environment routing'), causing duplicate test names and scattered coverage; consolidate routing tests by moving the env-mapping cases from Envilder.resolveFile (the tests named Should_ReturnEmpty_When_EnvMapsToNull and Should_ReturnEmpty_When_EnvNotInMapping) into the environment routing describe block or refactor both blocks into one describe('environment routing') that parameterizes over the two entry points (Envilder.resolveFile and Envilder.load), and rename duplicated tests to indicate which method is under test (e.g., ..._When_ResolveFileCalled vs ..._When_LoadCalled) so routing behavior for both resolveFile and load is colocated and uniquely identified.tests/sdks/typescript/containers/lowkey-vault-container.ts (2)
25-29: Hardcoded port8443duplicatesHTTPS_PORTconstant.If
HTTPS_PORTis ever changed, the--server.port=8443arg silently desynchronizes from the container's exposed port.🛠️ Proposed fix
this.container = await new GenericContainer(LOWKEY_VAULT_IMAGE) .withExposedPorts(HTTPS_PORT, HTTP_PORT) .withEnvironment({ - LOWKEY_ARGS: '--server.port=8443 --LOWKEY_VAULT_RELAXED_PORTS=true', + LOWKEY_ARGS: `--server.port=${HTTPS_PORT} --LOWKEY_VAULT_RELAXED_PORTS=true`, }) .start();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/sdks/typescript/containers/lowkey-vault-container.ts` around lines 25 - 29, The LOWKEY_ARGS environment value hardcodes --server.port=8443 which can drift from the HTTPS_PORT constant; update the withEnvironment call that sets LOWKEY_ARGS to build the server.port value from the HTTPS_PORT constant (e.g. use a template/string concatenation) so the value stays in sync with withExposedPorts(HTTPS_PORT, HTTP_PORT); adjust any necessary string conversion so LOWKEY_ARGS contains the numeric HTTPS_PORT at runtime.
91-108: Outertry/catchis unreachable — Promise never rejects.The inner
new Promise<boolean>always callsresolve(...)(on success, onerror, on timeout); it never rejects. The wrappingtry { ... } catch {}is dead code.🛠️ Proposed fix
for (let attempt = 0; attempt < MAX_RETRIES; attempt++) { - try { - const ok = await new Promise<boolean>((resolve) => { - const req = https.get(url, { agent }, (res) => { - resolve(res.statusCode === 200); - }); - req.on('error', () => resolve(false)); - req.setTimeout(2000, () => { - req.destroy(); - resolve(false); - }); - }); - if (ok) { - return; - } - } catch { - // retry - } + const ok = await new Promise<boolean>((resolve) => { + const req = https.get(url, { agent }, (res) => { + resolve(res.statusCode === 200); + }); + req.on('error', () => resolve(false)); + req.setTimeout(2000, () => { + req.destroy(); + resolve(false); + }); + }); + if (ok) { + return; + } if (attempt < MAX_RETRIES - 1) { await new Promise((r) => setTimeout(r, RETRY_DELAY_MS)); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/sdks/typescript/containers/lowkey-vault-container.ts` around lines 91 - 108, The outer try/catch around the await new Promise<boolean> is dead because the promise always resolves, so remove the try/catch and rely on the resolved boolean to decide retries: keep the loop using MAX_RETRIES, await the Promise created around https.get (the new Promise<boolean> that resolves via req, req.on('error'), and req.setTimeout), check the returned ok value and continue/retry if false, and only return when ok is true; or alternatively change the inner promise to reject on unexpected errors/timeouts instead of resolving so the try/catch around the await becomes meaningful—update references to MAX_RETRIES, the new Promise<boolean>, req, url, and agent accordingly.docs/architecture/adr/0001-sdk-acceptance-test-infrastructure.md (1)
75-75: TypeScript API version not documented.The "API version" row lists
7.2 for .NET, 7.6 for Pythonbut omits TypeScript, even though this PR ships the TypeScript implementation. Either add the TS-pinned version or note that TS uses the SDK default.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/architecture/adr/0001-sdk-acceptance-test-infrastructure.md` at line 75, The API version table row currently lists ".NET" and "Python" but omits TypeScript; update the "API version" row text to either include the pinned TypeScript version (e.g., add "TypeScript: X.Y" matching the shipped implementation) or explicitly state that TypeScript uses the SDK default (e.g., "TypeScript: uses SDK default") so the "API version" row is complete and unambiguous.tests/sdks/typescript/containers/localstack-container.ts (1)
25-30: Cascading failure withafterAllin the consuming test.When this throws (no
LOCALSTACK_AUTH_TOKENresolvable, e.g. forks/local without credentials),aws-ssm.acceptance.test.ts'sbeforeAllfails and itsafterAllthen dereferences an undefinedlocalstack(see the related comment in that file). The error here is the actionable one and gets buried.Consider also documenting in a brief comment that this hard-fails by design when the token can't be resolved, so contributors know to set up
secrets-map.jsonor skip the suite.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/sdks/typescript/containers/localstack-container.ts` around lines 25 - 30, The hard throw when LOCALSTACK_AUTH_TOKEN is missing causes downstream afterAll to dereference an undefined localstack; change the behavior in loadEnvironment / the block that checks environment.get('LOCALSTACK_AUTH_TOKEN') so it does not throw: instead return null/undefined (or a sentinel) from the function that creates the Localstack container (referenced in localstack-container.ts) and add a brief comment above the check explaining this is an intentional hard-fail/skip decision and that callers should handle a missing localstack (e.g., tests should skip when the returned value is falsy). Update callers to guard on the returned value (e.g., aws-ssm.acceptance.test.ts beforeAll/afterAll) so afterAll won’t dereference localstack when setup failed.tests/sdks/typescript/acceptance/aws-ssm.acceptance.test.ts (1)
8-10: Module-level shared state across test files.
localstack,ssmClient,providerare declared at module scope. If Vitest ever runs files in the same worker context, this is fine, but consider scoping insidedescribe(or using a small fixture object) to avoid accidental leakage and simplify future parallelization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/sdks/typescript/acceptance/aws-ssm.acceptance.test.ts` around lines 8 - 10, The tests declare module-level shared state (localstack, ssmClient, provider) which can leak between test files; move these variables into the describe block (or into a per-test fixture object) so each test suite gets its own instance, e.g. declare them inside the describe callback and initialize/cleanup in beforeAll/afterAll or beforeEach/afterEach using the existing setup/teardown logic for LocalStackTestContainer, SSMClient, and AwsSsmSecretProvider to avoid cross-file contamination and enable parallelization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/architecture/adr/0001-sdk-acceptance-test-infrastructure.md`:
- Around line 90-99: The fenced code block showing the directory tree (the
triple backticks before "tests/sdks/{lang}/" through the closing backticks)
lacks a language identifier and triggers MD040; update that fence (the opening
``` line) to include a language token such as "text" (e.g., change "```" to
"```text") so markdownlint accepts the block while preserving the directory-tree
content in the file
`docs/architecture/adr/0001-sdk-acceptance-test-infrastructure.md`.
In `@tests/sdks/typescript/acceptance/aws-ssm.acceptance.test.ts`:
- Around line 19-21: The afterAll teardown currently assumes localstack was
created and calls localstack.stop(), which throws if beforeAll failed and
localstack is undefined; change the afterAll handler (and the module-level
localstack declaration if needed) to guard the shutdown call — e.g., only call
stop when localstack is truthy (or use optional chaining) so the real error from
LocalStackTestContainer.start() surfaces instead of a TypeError; update
references to LocalStackTestContainer, beforeAll, and afterAll accordingly.
In `@tests/sdks/typescript/acceptance/azure-key-vault.acceptance.test.ts`:
- Around line 20-22: The afterAll teardown unconditionally calls
lowkeyVault.stop() which throws if LowkeyVaultTestContainer.start() failed and
lowkeyVault is undefined; update the afterAll to guard the call (e.g., if
(lowkeyVault) await lowkeyVault.stop();) or wrap in try/catch so teardown only
runs when lowkeyVault was successfully assigned; locate the lowkeyVault symbol
and the afterAll closure to apply this defensive check.
In `@tests/sdks/typescript/containers/lowkey-vault-container.ts`:
- Around line 47-50: The code captures
prevIdentityEndpoint/prevIdentityHeader/prevTlsRejectUnauthorized too late so a
thrown error from GenericContainer.start or waitUntilReady can cause this.stop()
to call restoreEnv and delete real env vars; fix by capturing those previous env
values before entering the try (or introduce an envOverridden boolean) and only
call restoreEnv for IDENTITY_ENDPOINT/IDENTITY_HEADER/TLS_REJECT_UNAUTHORIZED
when you actually set/overrode them; update the start logic around
GenericContainer.start and waitUntilReady to set envOverridden = true when
applying overrides and change this.stop to skip restoreEnv calls unless
envOverridden is true.
---
Nitpick comments:
In `@docs/architecture/adr/0001-sdk-acceptance-test-infrastructure.md`:
- Line 75: The API version table row currently lists ".NET" and "Python" but
omits TypeScript; update the "API version" row text to either include the pinned
TypeScript version (e.g., add "TypeScript: X.Y" matching the shipped
implementation) or explicitly state that TypeScript uses the SDK default (e.g.,
"TypeScript: uses SDK default") so the "API version" row is complete and
unambiguous.
In `@tests/sdks/typescript/acceptance/aws-ssm.acceptance.test.ts`:
- Around line 8-10: The tests declare module-level shared state (localstack,
ssmClient, provider) which can leak between test files; move these variables
into the describe block (or into a per-test fixture object) so each test suite
gets its own instance, e.g. declare them inside the describe callback and
initialize/cleanup in beforeAll/afterAll or beforeEach/afterEach using the
existing setup/teardown logic for LocalStackTestContainer, SSMClient, and
AwsSsmSecretProvider to avoid cross-file contamination and enable
parallelization.
In `@tests/sdks/typescript/application/envilder.test.ts`:
- Around line 99-110: Update the test
Should_ResolveFromMappedFile_When_EnvMappingProvided to assert both environment
variables are not injected: after calling Envilder.resolveFile('production',
...) and verifying mockReadFile was called, add an assertion that
process.env.API_KEY is undefined in addition to the existing process.env.DB_URL
check to mirror other resolve-only tests and prevent partial-injection
regressions; reference Envilder.resolveFile and mockReadFile in your change.
- Around line 51-56: Remove the redundant vi.restoreAllMocks() call from the
afterEach block in the test file: keep vi.clearAllMocks() and the environment
cleanup but delete vi.restoreAllMocks() because mocks are created via vi.mock()
and individual vi.fn() implementations such as mockProvider.getSecrets (and
other mocks like mockReadFile and mockCreateProvider) should not have their
implementations reverted by restoreAllMocks; this ensures each beforeEach can
reliably set mock implementations without unexpected resets.
- Around line 87-187: Tests for env-mapping behavior are split between
describe('resolveFile') and describe('environment routing'), causing duplicate
test names and scattered coverage; consolidate routing tests by moving the
env-mapping cases from Envilder.resolveFile (the tests named
Should_ReturnEmpty_When_EnvMapsToNull and
Should_ReturnEmpty_When_EnvNotInMapping) into the environment routing describe
block or refactor both blocks into one describe('environment routing') that
parameterizes over the two entry points (Envilder.resolveFile and
Envilder.load), and rename duplicated tests to indicate which method is under
test (e.g., ..._When_ResolveFileCalled vs ..._When_LoadCalled) so routing
behavior for both resolveFile and load is colocated and uniquely identified.
In `@tests/sdks/typescript/containers/localstack-container.ts`:
- Around line 25-30: The hard throw when LOCALSTACK_AUTH_TOKEN is missing causes
downstream afterAll to dereference an undefined localstack; change the behavior
in loadEnvironment / the block that checks
environment.get('LOCALSTACK_AUTH_TOKEN') so it does not throw: instead return
null/undefined (or a sentinel) from the function that creates the Localstack
container (referenced in localstack-container.ts) and add a brief comment above
the check explaining this is an intentional hard-fail/skip decision and that
callers should handle a missing localstack (e.g., tests should skip when the
returned value is falsy). Update callers to guard on the returned value (e.g.,
aws-ssm.acceptance.test.ts beforeAll/afterAll) so afterAll won’t dereference
localstack when setup failed.
In `@tests/sdks/typescript/containers/lowkey-vault-container.ts`:
- Around line 25-29: The LOWKEY_ARGS environment value hardcodes
--server.port=8443 which can drift from the HTTPS_PORT constant; update the
withEnvironment call that sets LOWKEY_ARGS to build the server.port value from
the HTTPS_PORT constant (e.g. use a template/string concatenation) so the value
stays in sync with withExposedPorts(HTTPS_PORT, HTTP_PORT); adjust any necessary
string conversion so LOWKEY_ARGS contains the numeric HTTPS_PORT at runtime.
- Around line 91-108: The outer try/catch around the await new Promise<boolean>
is dead because the promise always resolves, so remove the try/catch and rely on
the resolved boolean to decide retries: keep the loop using MAX_RETRIES, await
the Promise created around https.get (the new Promise<boolean> that resolves via
req, req.on('error'), and req.setTimeout), check the returned ok value and
continue/retry if false, and only return when ok is true; or alternatively
change the inner promise to reject on unexpected errors/timeouts instead of
resolving so the try/catch around the await becomes meaningful—update references
to MAX_RETRIES, the new Promise<boolean>, req, url, and agent accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d5c06f69-8550-471b-af95-c347db72ef3b
⛔ Files ignored due to path filters (5)
.github/skills/sdk-acceptance-testing/SKILL.mdis excluded by none and included by none.github/workflows/coverage-report.ymlis excluded by none and included by none.github/workflows/publish-npm-sdk.ymlis excluded by none and included by none.github/workflows/tests-typescript-sdk.ymlis excluded by none and included by nonepnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yamland included by none
📒 Files selected for processing (8)
docs/architecture/adr/0001-sdk-acceptance-test-infrastructure.mdtests/sdks/typescript/acceptance/aws-ssm.acceptance.test.tstests/sdks/typescript/acceptance/azure-key-vault.acceptance.test.tstests/sdks/typescript/application/envilder.test.tstests/sdks/typescript/application/map-file-parser.test.tstests/sdks/typescript/containers/localstack-container.tstests/sdks/typescript/containers/lowkey-vault-container.tstests/sdks/typescript/package.json
✅ Files skipped from review due to trivial changes (1)
- tests/sdks/typescript/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/sdks/typescript/application/map-file-parser.test.ts
Comment on lines
+90
to
+99
| ``` | ||
| tests/sdks/{lang}/ | ||
| ├── containers/ ← Container wrappers | ||
| │ ├── localstack-container | ||
| │ └── lowkey-vault-container | ||
| ├── acceptance/ ← Acceptance tests | ||
| │ ├── aws-ssm.acceptance.* | ||
| │ └── azure-key-vault.acceptance.* | ||
| └── {unit tests}/ | ||
| ``` |
Contributor
There was a problem hiding this comment.
Fix MD040: fenced code block needs a language identifier.
This is the failure reported by markdownlint in the Rainbow Road Lint job.
🛠️ Proposed fix
-```
+```text
tests/sdks/{lang}/
├── containers/ ← Container wrappers
│ ├── localstack-container
│ └── lowkey-vault-container
├── acceptance/ ← Acceptance tests
│ ├── aws-ssm.acceptance.*
│ └── azure-key-vault.acceptance.*
└── {unit tests}/
```🧰 Tools
🪛 GitHub Actions: 🏁 Rainbow Road Lint
[error] 90-90: markdownlint failed (MD040/fenced-code-language): Fenced code blocks should have a language specified [Context: "```"].
🪛 GitHub Check: markdown-lint
[failure] 90-90:
MD040/fenced-code-language Fenced code blocks should have a language specified [Context: "```"]
🪛 markdownlint-cli2 (0.22.1)
[warning] 90-90: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/architecture/adr/0001-sdk-acceptance-test-infrastructure.md` around
lines 90 - 99, The fenced code block showing the directory tree (the triple
backticks before "tests/sdks/{lang}/" through the closing backticks) lacks a
language identifier and triggers MD040; update that fence (the opening ``` line)
to include a language token such as "text" (e.g., change "```" to "```text") so
markdownlint accepts the block while preserving the directory-tree content in
the file `docs/architecture/adr/0001-sdk-acceptance-test-infrastructure.md`.
Comment on lines
+19
to
+21
| afterAll(async () => { | ||
| await localstack.stop(); | ||
| }); |
Contributor
There was a problem hiding this comment.
afterAll crashes when beforeAll fails — masks the real error.
The pipeline is failing here with TypeError: Cannot read properties of undefined (reading 'stop'). When LocalStackTestContainer.start() throws (e.g. LOCALSTACK_AUTH_TOKEN not resolvable in CI), the module-level localstack stays undefined, and the dereference in afterAll masks the original failure cause and reports a confusing error.
Guard the teardown so the underlying setup error surfaces.
🛠️ Proposed fix
afterAll(async () => {
- await localstack.stop();
+ await localstack?.stop();
});📝 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
| afterAll(async () => { | |
| await localstack.stop(); | |
| }); | |
| afterAll(async () => { | |
| await localstack?.stop(); | |
| }); |
🧰 Tools
🪛 GitHub Actions: 🟦 TypeScript SDK Tests
[error] 20-20: TypeError: Cannot read properties of undefined (reading 'stop'). at tests/sdks/typescript/acceptance/aws-ssm.acceptance.test.ts:20:22 (afterAll calls await localstack.stop()).
[error] Vitest failed suite: Failed Suites 1 (tests/sdks/typescript/acceptance/aws-ssm.acceptance.test.ts > AWS SSM Acceptance).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/sdks/typescript/acceptance/aws-ssm.acceptance.test.ts` around lines 19
- 21, The afterAll teardown currently assumes localstack was created and calls
localstack.stop(), which throws if beforeAll failed and localstack is undefined;
change the afterAll handler (and the module-level localstack declaration if
needed) to guard the shutdown call — e.g., only call stop when localstack is
truthy (or use optional chaining) so the real error from
LocalStackTestContainer.start() surfaces instead of a TypeError; update
references to LocalStackTestContainer, beforeAll, and afterAll accordingly.
Comment on lines
+20
to
+22
| afterAll(async () => { | ||
| await lowkeyVault.stop(); | ||
| }); |
Contributor
There was a problem hiding this comment.
Same teardown hazard as aws-ssm.acceptance.test.ts.
If LowkeyVaultTestContainer.start() fails (Docker unavailable, image pull failure, readiness timeout), lowkeyVault stays undefined and afterAll will throw TypeError: Cannot read properties of undefined (reading 'stop'), hiding the real setup error.
🛠️ Proposed fix
afterAll(async () => {
- await lowkeyVault.stop();
+ await lowkeyVault?.stop();
});📝 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
| afterAll(async () => { | |
| await lowkeyVault.stop(); | |
| }); | |
| afterAll(async () => { | |
| await lowkeyVault?.stop(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/sdks/typescript/acceptance/azure-key-vault.acceptance.test.ts` around
lines 20 - 22, The afterAll teardown unconditionally calls lowkeyVault.stop()
which throws if LowkeyVaultTestContainer.start() failed and lowkeyVault is
undefined; update the afterAll to guard the call (e.g., if (lowkeyVault) await
lowkeyVault.stop();) or wrap in try/catch so teardown only runs when lowkeyVault
was successfully assigned; locate the lowkeyVault symbol and the afterAll
closure to apply this defensive check.
Comment on lines
+47
to
+50
| } catch (error) { | ||
| await this.stop(); | ||
| throw error; | ||
| } |
Contributor
There was a problem hiding this comment.
Failure path can clobber pre-existing env vars.
prevIdentityEndpoint / prevIdentityHeader / prevTlsRejectUnauthorized are captured on lines 40–42, immediately before the overrides. If GenericContainer.start() (line 24) or waitUntilReady() (line 38) throws, control jumps to the catch and calls this.stop(), whose finally runs restoreEnv('IDENTITY_ENDPOINT', undefined) etc. with previous still undefined. restoreEnv then deletes those env vars from process.env — even though we never overrode them — wiping out any pre-existing values the user had set (e.g., real Azure identity config in a dev environment).
Capture the previous values before the try (or only restore when an override was actually applied).
🛠️ Proposed fix
async start(): Promise<LowkeyVaultTestContainer> {
console.log('\n[LowkeyVault] Starting container...');
+ this.prevIdentityEndpoint = process.env.IDENTITY_ENDPOINT;
+ this.prevIdentityHeader = process.env.IDENTITY_HEADER;
+ this.prevTlsRejectUnauthorized = process.env.NODE_TLS_REJECT_UNAUTHORIZED;
+ let envOverridden = false;
+
try {
this.container = await new GenericContainer(LOWKEY_VAULT_IMAGE)
...
await this.waitUntilReady();
- this.prevIdentityEndpoint = process.env.IDENTITY_ENDPOINT;
- this.prevIdentityHeader = process.env.IDENTITY_HEADER;
- this.prevTlsRejectUnauthorized = process.env.NODE_TLS_REJECT_UNAUTHORIZED;
-
process.env.IDENTITY_ENDPOINT = tokenUrl;
process.env.IDENTITY_HEADER = 'dummy';
process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0';
+ envOverridden = true;
} catch (error) {
- await this.stop();
+ await this.stop(envOverridden);
throw error;
}(and have stop skip restoreEnv calls when envOverridden === false).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/sdks/typescript/containers/lowkey-vault-container.ts` around lines 47 -
50, The code captures
prevIdentityEndpoint/prevIdentityHeader/prevTlsRejectUnauthorized too late so a
thrown error from GenericContainer.start or waitUntilReady can cause this.stop()
to call restoreEnv and delete real env vars; fix by capturing those previous env
values before entering the try (or introduce an envOverridden boolean) and only
call restoreEnv for IDENTITY_ENDPOINT/IDENTITY_HEADER/TLS_REJECT_UNAUTHORIZED
when you actually set/overrode them; update the start logic around
GenericContainer.start and waitUntilReady to set envOverridden = true when
applying overrides and change this.stop to skip restoreEnv calls unless
envOverridden is true.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new TypeScript runtime SDK (@envilder/sdk) to the monorepo (layered domain/application/infrastructure), along with a dedicated Vitest test workspace (unit + acceptance tests), CI coverage integration, publish workflow, and website/docs updates to announce/support the SDK.
Changes:
- Add TypeScript SDK source package (
src/sdks/typescript) with AWS SSM + Azure Key Vault providers, factory, facade/builder, parser, and validation. - Add TypeScript SDK test workspace (
tests/sdks/typescript) with Vitest config, unit tests, and TestContainers-based acceptance tests (LocalStack + Lowkey Vault). - Update website/docs/i18n + changelog infra, CI workflows (tests, coverage-report integration), pnpm workspace, and repo tooling scripts/hooks.
Reviewed changes
Copilot reviewed 53 out of 55 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/sdks/typescript/vitest.config.ts | Adds Vitest config for the TS SDK test workspace (coverage + alias). |
| tests/sdks/typescript/tsconfig.json | Adds TS config for the TS SDK test workspace. |
| tests/sdks/typescript/package.json | Adds test workspace package + Vitest scripts/deps. |
| tests/sdks/typescript/infrastructure/secret-provider-factory.test.ts | Unit tests for provider factory behavior. |
| tests/sdks/typescript/infrastructure/azure/azure-key-vault-secret-provider.test.ts | Unit tests for Azure Key Vault provider. |
| tests/sdks/typescript/infrastructure/aws/aws-ssm-secret-provider.test.ts | Unit tests for AWS SSM provider. |
| tests/sdks/typescript/containers/lowkey-vault-container.ts | Lowkey Vault container wrapper for acceptance tests. |
| tests/sdks/typescript/containers/localstack-container.ts | LocalStack container wrapper for acceptance tests (resolves LocalStack token). |
| tests/sdks/typescript/application/secret-validation.test.ts | Unit tests for validateSecrets() and SecretValidationError. |
| tests/sdks/typescript/application/map-file-parser.test.ts | Unit tests for map-file parsing (mappings + $config). |
| tests/sdks/typescript/application/envilder.test.ts | Unit tests for facade overloads, env routing, and fluent builder overrides. |
| tests/sdks/typescript/application/envilder-client.test.ts | Unit tests for core resolver + environment injection. |
| tests/sdks/typescript/acceptance/azure-key-vault.acceptance.test.ts | Acceptance tests against Lowkey Vault (real SDK client/provider flow). |
| tests/sdks/typescript/acceptance/aws-ssm.acceptance.test.ts | Acceptance tests against LocalStack SSM (real provider calls). |
| tests/sdks/typescript/.gitkeep | Removes placeholder file for the TS SDK tests folder. |
| src/website/src/pages/es/changelog.astro | Adds TypeScript SDK changelog category to Spanish page. |
| src/website/src/pages/changelog.astro | Adds TypeScript SDK changelog category to default page. |
| src/website/src/pages/ca/changelog.astro | Adds TypeScript SDK changelog category to Catalan page. |
| src/website/src/i18n/types.ts | Adds translation keys for TypeScript SDK docs/changelog/sidebar entries. |
| src/website/src/i18n/es.ts | Updates TS SDK status + adds docs/changelog translations (ES). |
| src/website/src/i18n/en.ts | Updates TS SDK status + adds docs/changelog translations (EN). |
| src/website/src/i18n/ca.ts | Updates TS SDK status + adds docs/changelog translations (CA). |
| src/website/src/env.d.ts | Adds global changelog constant for TypeScript SDK. |
| src/website/src/components/Sdks.astro | Marks TS SDK as available and adds install/docs/code snippet on landing page. |
| src/website/src/components/DocsContent.astro | Adds a full TypeScript SDK docs section (install, quickstart, fluent, env routing, validation). |
| src/website/astro.config.mjs | Reads + injects TypeScript SDK changelog content into build defines. |
| src/sdks/typescript/tsconfig.json | Adds TS SDK build config (Node16 module resolution, declarations, sourcemaps). |
| src/sdks/typescript/src/infrastructure/secret-provider-factory.ts | Adds provider factory (createSecretProvider) with cross-provider option validation. |
| src/sdks/typescript/src/infrastructure/azure/azure-key-vault-secret-provider.ts | Adds Azure Key Vault provider implementation (parallel fetch, 404 omission). |
| src/sdks/typescript/src/infrastructure/aws/aws-ssm-secret-provider.ts | Adds AWS SSM provider implementation (GetParameters batching + decryption). |
| src/sdks/typescript/src/index.ts | Adds public barrel exports for the SDK (facade, client, parser, validation, types, providers). |
| src/sdks/typescript/src/domain/secret-provider-type.ts | Adds provider type enum (aws/azure). |
| src/sdks/typescript/src/domain/ports/secret-provider.ts | Adds provider port interface (getSecrets). |
| src/sdks/typescript/src/domain/parsed-map-file.ts | Adds parsed map-file type (config + mappings). |
| src/sdks/typescript/src/domain/map-file-config.ts | Adds map-file config type (provider/vaultUrl/profile). |
| src/sdks/typescript/src/domain/envilder-options.ts | Adds runtime override options type (provider/vaultUrl/profile). |
| src/sdks/typescript/src/application/secret-validation.ts | Adds validateSecrets() and SecretValidationError. |
| src/sdks/typescript/src/application/map-file-parser.ts | Adds JSON map-file parser supporting $config and provider validation. |
| src/sdks/typescript/src/application/envilder.ts | Adds main facade + fluent builder + env routing overloads. |
| src/sdks/typescript/src/application/envilder-client.ts | Adds core resolver and process.env injector. |
| src/sdks/typescript/package.json | Adds publishable npm package manifest for @envilder/sdk. |
| src/sdks/typescript/.gitkeep | Removes placeholder file for the TS SDK source folder. |
| pnpm-workspace.yaml | Adds TS SDK source + tests workspaces to pnpm workspace. |
| pnpm-lock.yaml | Locks dependencies for the new workspaces. |
| package.json | Updates root scripts (format, format:check, lint) behavior. |
| lefthook.yml | Expands Biome file glob and runs both check+format on staged files. |
| docs/changelogs/sdk-typescript.md | Adds TypeScript SDK changelog file for website integration. |
| docs/architecture/adr/0001-sdk-acceptance-test-infrastructure.md | Documents acceptance test infra pattern across SDKs (ADR). |
| ROADMAP.md | Moves TypeScript SDK from “Up Next” to shipped list. |
| .github/workflows/tests-typescript-sdk.yml | Adds dedicated CI workflow for TS SDK tests. |
| .github/workflows/publish-npm-sdk.yml | Adds npm publish workflow for the TypeScript SDK package. |
| .github/workflows/coverage-report/coverage-config.json | Adds coverage thresholds entry for “typescript-sdk”. |
| .github/workflows/coverage-report.yml | Adds a TypeScript SDK coverage job and includes it in aggregated dashboard. |
| .github/skills/sdk-acceptance-testing/SKILL.md | Adds a skill doc describing SDK acceptance testing patterns. |
| .github/copilot-instructions.md | Documents TypeScript SDK architecture/testing conventions for contributors. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comment on lines
+21
to
+29
| if (isAzure && profile) { | ||
| throw new Error( | ||
| 'AWS profile cannot be used with Azure Key Vault provider.', | ||
| ); | ||
| } | ||
|
|
||
| if (!isAzure && vaultUrl) { | ||
| throw new Error('Vault URL cannot be used with AWS SSM provider.'); | ||
| } |
|
|
||
| for (const name of names) { | ||
| if (!name?.trim()) { | ||
| throw new Error('Secret name cannot be null or empty.'); |
|
|
||
| for (const name of names) { | ||
| if (!name?.trim()) { | ||
| throw new Error('Secret name cannot be null or whitespace.'); |
Comment on lines
+49
to
+54
| const actual = createSecretProvider(config); | ||
|
|
||
| // Assert | ||
| expect(actual).toBeDefined(); | ||
| expect(actual.getSecret).toBeDefined(); | ||
| }); |
Comment on lines
+14
to
+17
| "paths": { | ||
| "@envilder/sdk": ["../../src/sdks/typescript/src/index.js"], | ||
| "@envilder/sdk/*": ["../../src/sdks/typescript/src/*"] | ||
| } |
Comment on lines
+12
to
+16
| super( | ||
| missingKeys.length === 0 | ||
| ? 'No secrets were resolved.' | ||
| : `The following secrets have empty or missing values: ${missingKeys.join(', ')}`, | ||
| ); |
| vaultUrl: string | undefined, | ||
| ): AzureKeyVaultSecretProvider { | ||
| if (!vaultUrl?.trim()) { | ||
| throw new Error('Vault URL must be provided for Azure Key Vault provider.'); |
| ); | ||
|
|
||
| for (const param of response.Parameters ?? []) { | ||
| if (param.Name && param.Value) { |
| "lint": "secretlint \"**/*\" && biome check --write && tsc --noEmit", | ||
| "format": "biome check --write --unsafe && biome format --write", | ||
| "format:check": "biome check && biome format", | ||
| "lint": "secretlint \"**/*\" && biome check --write --unsafe && tsc --noEmit", |
macalbert
had a problem deploying
to
github-pages
GitHub Actions
Failure
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/sdks/typescript/src/application/envilder.ts (1)
95-111: Minor: fluent setters mutate shared per-instance options.
withProvider/withVaultUrl/withProfilemutatethis.optionson the sameEnvilderinstance, so reusing an instance across calls (e.g. resolving twice with different overrides) carries state. This matches the “fluent builder” style documented in the JSDoc, but if you ever want to support immutable composition in the future it would be cleaner to either return a newEnvilderper call or freeze options after the firstresolve(). Not blocking — just flagging for the public API contract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sdks/typescript/src/application/envilder.ts` around lines 95 - 111, The fluent setters withProvider, withVaultUrl, and withProfile currently mutate this.options on the same Envilder instance which causes shared mutable state; change each setter to return a new Envilder with a cloned options object (e.g. shallow copy of this.options plus the updated field) instead of mutating this.options, or alternatively implement a copy constructor used by the setters that builds a new instance with copied options; additionally, consider freezing options after resolve() to prevent further mutation if immutability is desired.src/sdks/typescript/src/infrastructure/aws/aws-ssm-secret-provider.ts (1)
37-51: Optional: fetch SSM batches in parallel.Batches are awaited sequentially, so 30 names → 3 round-trips serialized. For larger map files this latency adds up. Consider issuing the
GetParametersCommandcalls concurrently withPromise.allwhile preserving the merge order, since the result is keyed byparam.Name.♻️ Proposed refactor
- for (let i = 0; i < names.length; i += SSM_BATCH_SIZE) { - const batch = names.slice(i, i + SSM_BATCH_SIZE); - const response = await this.ssmClient.send( - new GetParametersCommand({ - Names: batch, - WithDecryption: true, - }), - ); - - for (const param of response.Parameters ?? []) { - if (param.Name && param.Value) { - result.set(param.Name, param.Value); - } - } - } + const batches: string[][] = []; + for (let i = 0; i < names.length; i += SSM_BATCH_SIZE) { + batches.push(names.slice(i, i + SSM_BATCH_SIZE)); + } + const responses = await Promise.all( + batches.map((batch) => + this.ssmClient.send( + new GetParametersCommand({ Names: batch, WithDecryption: true }), + ), + ), + ); + for (const response of responses) { + for (const param of response.Parameters ?? []) { + if (param.Name && param.Value) { + result.set(param.Name, param.Value); + } + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sdks/typescript/src/infrastructure/aws/aws-ssm-secret-provider.ts` around lines 37 - 51, The batch GetParametersCommand calls in aws-ssm-secret-provider.ts are executed sequentially; change the logic in the method that uses SSM_BATCH_SIZE to build an array of promises calling this.ssmClient.send(new GetParametersCommand({ Names: batch, WithDecryption: true })) for each batch, await them with Promise.all, then iterate the combined responses and populate the result Map (using param.Name and param.Value) — keep using SSM_BATCH_SIZE, this.ssmClient.send, GetParametersCommand, and the existing result.set(...) merging by param.Name so concurrency doesn’t change final contents.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/sdks/typescript/src/application/envilder.ts`:
- Around line 56-93: The TypeScript SDK currently throws in resolveEnvSource
when an environment key is missing; update resolveEnvSource so that when the
normalized env key is not found in the provided envMapping it returns null (not
throw) to preserve the "missing/null mapping → empty dict" behavior used by load
and resolveFile; locate the resolveEnvSource function and replace the exception
path with a return null, keeping any normalization/trimming logic intact so
callers (Envilder.load and Envilder.resolveFile) can detect null and return an
empty Map.
In `@src/sdks/typescript/src/infrastructure/secret-provider-factory.ts`:
- Around line 21-25: The multi-line throw inside the if (isAzure && profile)
block should be collapsed to a single line to satisfy Biome formatting; replace
the current block's body with a single-line throw: throw new Error('AWS profile
cannot be used with Azure Key Vault provider'); ensuring single quotes and a
terminating semicolon, leaving the if condition (isAzure && profile) intact.
In `@tests/sdks/typescript/containers/localstack-container.ts`:
- Around line 37-40: The test hardcodes the Docker container name by calling
.withName('localstack') on the LocalstackContainer so repeated runs can fail
with "container name already in use"; remove the explicit .withName call (or
replace it with a unique name generator / suffix) where the LocalstackContainer
is created so Testcontainers can assign a unique name (or the name is suffixed
with a random/UUID value) before calling .start(); update the
LocalstackContainer instantiation site accordingly (the expression creating
this.container) to avoid fixed container names.
---
Nitpick comments:
In `@src/sdks/typescript/src/application/envilder.ts`:
- Around line 95-111: The fluent setters withProvider, withVaultUrl, and
withProfile currently mutate this.options on the same Envilder instance which
causes shared mutable state; change each setter to return a new Envilder with a
cloned options object (e.g. shallow copy of this.options plus the updated field)
instead of mutating this.options, or alternatively implement a copy constructor
used by the setters that builds a new instance with copied options;
additionally, consider freezing options after resolve() to prevent further
mutation if immutability is desired.
In `@src/sdks/typescript/src/infrastructure/aws/aws-ssm-secret-provider.ts`:
- Around line 37-51: The batch GetParametersCommand calls in
aws-ssm-secret-provider.ts are executed sequentially; change the logic in the
method that uses SSM_BATCH_SIZE to build an array of promises calling
this.ssmClient.send(new GetParametersCommand({ Names: batch, WithDecryption:
true })) for each batch, await them with Promise.all, then iterate the combined
responses and populate the result Map (using param.Name and param.Value) — keep
using SSM_BATCH_SIZE, this.ssmClient.send, GetParametersCommand, and the
existing result.set(...) merging by param.Name so concurrency doesn’t change
final contents.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ebb99252-a67d-4d5c-b93f-c01b0ef50d1b
⛔ Files ignored due to path filters (1)
.github/copilot-instructions.mdis excluded by none and included by none
📒 Files selected for processing (11)
src/sdks/typescript/package.jsonsrc/sdks/typescript/src/application/envilder.tssrc/sdks/typescript/src/application/secret-validation.tssrc/sdks/typescript/src/infrastructure/aws/aws-ssm-secret-provider.tssrc/sdks/typescript/src/infrastructure/azure/azure-key-vault-secret-provider.tssrc/sdks/typescript/src/infrastructure/secret-provider-factory.tstests/sdks/typescript/application/envilder.test.tstests/sdks/typescript/containers/localstack-container.tstests/sdks/typescript/containers/lowkey-vault-container.tstests/sdks/typescript/tsconfig.jsontests/sdks/typescript/vitest.config.ts
✅ Files skipped from review due to trivial changes (5)
- tests/sdks/typescript/tsconfig.json
- src/sdks/typescript/package.json
- tests/sdks/typescript/vitest.config.ts
- tests/sdks/typescript/application/envilder.test.ts
- src/sdks/typescript/src/infrastructure/azure/azure-key-vault-secret-provider.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/sdks/typescript/src/application/secret-validation.ts
- tests/sdks/typescript/containers/lowkey-vault-container.ts
Comment on lines
+56
to
+93
| static async load( | ||
| filePathOrEnv: string, | ||
| envMapping?: Record<string, string | null>, | ||
| ): Promise<Map<string, string>> { | ||
| if (envMapping !== undefined) { | ||
| const source = resolveEnvSource(filePathOrEnv, envMapping); | ||
| if (source === null) { | ||
| return new Map(); | ||
| } | ||
| return new Envilder(source).inject(); | ||
| } | ||
|
|
||
| validateFilePath(filePathOrEnv); | ||
| return new Envilder(filePathOrEnv.trim()).inject(); | ||
| } | ||
|
|
||
| /** | ||
| * Resolves secrets without injecting into `process.env`. | ||
| * | ||
| * Can be called in two ways: | ||
| * - `resolveFile(filePath)` — resolve from a single map file | ||
| * - `resolveFile(env, envMapping)` — look up env in the mapping | ||
| */ | ||
| static async resolveFile( | ||
| filePathOrEnv: string, | ||
| envMapping?: Record<string, string | null>, | ||
| ): Promise<Map<string, string>> { | ||
| if (envMapping !== undefined) { | ||
| const source = resolveEnvSource(filePathOrEnv, envMapping); | ||
| if (source === null) { | ||
| return new Map(); | ||
| } | ||
| return new Envilder(source).resolve(); | ||
| } | ||
|
|
||
| validateFilePath(filePathOrEnv); | ||
| return new Envilder(filePathOrEnv.trim()).resolve(); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare env-routing semantics across SDKs.
fd -t f 'Envilder.cs' src/sdks/dotnet | xargs -I{} sed -n '1,260p' {}
fd -t f 'envilder.py' src/sdks/python | xargs -I{} sed -n '1,260p' {}Repository: macalbert/envilder
Length of output: 11848
🏁 Script executed:
fd -t f 'envilder.ts' src/sdks/typescript | head -1 | xargs catRepository: macalbert/envilder
Length of output: 5219
🏁 Script executed:
fd -t f 'envilder.py' src/sdks/python | head -1 | xargs catRepository: macalbert/envilder
Length of output: 44
🏁 Script executed:
fd -t f '__init__.py' src/sdks/python/application | head -1 | xargs catRepository: macalbert/envilder
Length of output: 177
🏁 Script executed:
find src/sdks/python -name '*.py' -type f | grep -i envilder | head -5Repository: macalbert/envilder
Length of output: 316
🏁 Script executed:
find src/sdks/python -name '*.py' -type f | head -10Repository: macalbert/envilder
Length of output: 572
🏁 Script executed:
cat src/sdks/python/envilder/application/envilder_facade.pyRepository: macalbert/envilder
Length of output: 8524
TypeScript SDK does not maintain behavioral parity with .NET and Python SDKs for missing environment keys.
The TypeScript resolveEnvSource throws an error when the environment key is not found in the mapping (Environment '${normalized}' not found in environment mapping.), while both .NET and Python return null to allow the caller to handle missing environments as intentional skips. This breaks the designed "missing/null mapping → empty dict" pattern. Update TypeScript's resolveEnvSource to return null (instead of throwing) when the environment key is not found, matching the behavior of the other SDKs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/sdks/typescript/src/application/envilder.ts` around lines 56 - 93, The
TypeScript SDK currently throws in resolveEnvSource when an environment key is
missing; update resolveEnvSource so that when the normalized env key is not
found in the provided envMapping it returns null (not throw) to preserve the
"missing/null mapping → empty dict" behavior used by load and resolveFile;
locate the resolveEnvSource function and replace the exception path with a
return null, keeping any normalization/trimming logic intact so callers
(Envilder.load and Envilder.resolveFile) can detect null and return an empty
Map.
Comment on lines
+37
to
+40
| this.container = await new LocalstackContainer(LOCALSTACK_IMAGE) | ||
| .withName('localstack') | ||
| .withEnvironment(envRecord) | ||
| .start(); |
Contributor
There was a problem hiding this comment.
Hardcoded container name may break re-runs.
.withName('localstack') assigns a fixed Docker container name. If a previous run was interrupted (or another acceptance suite reuses the name on the same host/CI runner), the next start() will fail with "container name already in use". Either drop the explicit name (Testcontainers generates a unique name and still cleans up via Ryuk) or scope it with a random suffix.
🛠 Proposed fix
- this.container = await new LocalstackContainer(LOCALSTACK_IMAGE)
- .withName('localstack')
- .withEnvironment(envRecord)
- .start();
+ this.container = await new LocalstackContainer(LOCALSTACK_IMAGE)
+ .withEnvironment(envRecord)
+ .start();📝 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
| this.container = await new LocalstackContainer(LOCALSTACK_IMAGE) | |
| .withName('localstack') | |
| .withEnvironment(envRecord) | |
| .start(); | |
| this.container = await new LocalstackContainer(LOCALSTACK_IMAGE) | |
| .withEnvironment(envRecord) | |
| .start(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/sdks/typescript/containers/localstack-container.ts` around lines 37 -
40, The test hardcodes the Docker container name by calling
.withName('localstack') on the LocalstackContainer so repeated runs can fail
with "container name already in use"; remove the explicit .withName call (or
replace it with a unique name generator / suffix) where the LocalstackContainer
is created so Testcontainers can assign a unique name (or the name is suffixed
with a random/UUID value) before calling .start(); update the
LocalstackContainer instantiation site accordingly (the expression creating
this.container) to avoid fixed container names.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new standalone TypeScript runtime SDK (@envilder/sdk) to Envilder, plus test infrastructure, CI workflows, and website/docs wiring so the SDK is shipped and visible alongside the .NET and Python SDKs.
Changes:
- Introduces the TypeScript SDK (domain/application/infrastructure layers) with AWS SSM + Azure Key Vault providers, fluent facade, map parsing, and optional validation.
- Adds a dedicated Vitest test workspace including unit + acceptance tests (LocalStack + Lowkey Vault container wrappers).
- Wires the SDK into the website (docs, changelog, i18n, version badges) and CI (test + coverage + npm publish).
Reviewed changes
Copilot reviewed 56 out of 58 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/sdks/typescript/vitest.config.ts | Vitest config for TS SDK tests (root, coverage, alias). |
| tests/sdks/typescript/tsconfig.json | TS config for the TS SDK test workspace (paths + strict). |
| tests/sdks/typescript/package.json | Test workspace package and dev dependencies. |
| tests/sdks/typescript/infrastructure/secret-provider-factory.test.ts | Unit tests for provider factory behavior. |
| tests/sdks/typescript/infrastructure/azure/azure-key-vault-secret-provider.test.ts | Unit tests for Azure provider behavior. |
| tests/sdks/typescript/infrastructure/aws/aws-ssm-secret-provider.test.ts | Unit tests for AWS provider behavior. |
| tests/sdks/typescript/containers/lowkey-vault-container.ts | Lowkey Vault testcontainer wrapper for acceptance tests. |
| tests/sdks/typescript/containers/localstack-container.ts | LocalStack testcontainer wrapper + token resolution. |
| tests/sdks/typescript/application/secret-validation.test.ts | Unit tests for validateSecrets. |
| tests/sdks/typescript/application/map-file-parser.test.ts | Unit tests for map-file parsing. |
| tests/sdks/typescript/application/envilder.test.ts | Unit tests for the Envilder facade + fluent builder. |
| tests/sdks/typescript/application/envilder-client.test.ts | Unit tests for the core resolver/injector. |
| tests/sdks/typescript/acceptance/azure-key-vault.acceptance.test.ts | Acceptance tests against Lowkey Vault. |
| tests/sdks/typescript/acceptance/aws-ssm.acceptance.test.ts | Acceptance tests against LocalStack SSM. |
| tests/sdks/typescript/.gitkeep | Removes placeholder file now that tests exist. |
| src/website/src/pages/es/changelog.astro | Adds TS SDK to Spanish changelog navigation/products. |
| src/website/src/pages/changelog.astro | Adds TS SDK to default (en) changelog navigation/products. |
| src/website/src/pages/ca/changelog.astro | Adds TS SDK to Catalan changelog navigation/products. |
| src/website/src/i18n/types.ts | Adds i18n keys for TS SDK docs + changelog category. |
| src/website/src/i18n/es.ts | Spanish translations + roadmap status update for TS SDK. |
| src/website/src/i18n/en.ts | English translations + roadmap status update for TS SDK. |
| src/website/src/i18n/ca.ts | Catalan translations + roadmap status update for TS SDK. |
| src/website/src/env.d.ts | Declares new changelog/version build-time globals. |
| src/website/src/components/Sdks.astro | Marks TS SDK as available + adds install/docs/code sample. |
| src/website/src/components/DocsContent.astro | Adds TS SDK docs section + dynamic version badges. |
| src/website/astro.config.mjs | Reads TS SDK package version + defines new globals. |
| src/sdks/typescript/tsconfig.json | TS SDK build config (Node16 ESM, declarations, dist). |
| src/sdks/typescript/src/infrastructure/secret-provider-factory.ts | Provider factory (AWS/Azure selection + validation). |
| src/sdks/typescript/src/infrastructure/azure/azure-key-vault-secret-provider.ts | Azure Key Vault provider implementation. |
| src/sdks/typescript/src/infrastructure/aws/aws-ssm-secret-provider.ts | AWS SSM provider implementation (batch GetParameters). |
| src/sdks/typescript/src/index.ts | Public barrel exports for the TS SDK package. |
| src/sdks/typescript/src/domain/secret-provider-type.ts | Provider enum for AWS/Azure. |
| src/sdks/typescript/src/domain/ports/secret-provider.ts | ISecretProvider port definition. |
| src/sdks/typescript/src/domain/parsed-map-file.ts | Parsed map-file type. |
| src/sdks/typescript/src/domain/map-file-config.ts | $config shape for map files. |
| src/sdks/typescript/src/domain/envilder-options.ts | Runtime override options shape. |
| src/sdks/typescript/src/application/secret-validation.ts | validateSecrets + SecretValidationError. |
| src/sdks/typescript/src/application/map-file-parser.ts | JSON map-file parser ($config + mappings). |
| src/sdks/typescript/src/application/envilder.ts | Facade API (load/resolve/env routing/fluent builder). |
| src/sdks/typescript/src/application/envilder-client.ts | Core resolver and environment injector. |
| src/sdks/typescript/package.json | TS SDK package metadata, deps, build scripts, exports. |
| src/sdks/typescript/.gitkeep | Removes placeholder file now that SDK exists. |
| pnpm-workspace.yaml | Adds TS SDK + TS SDK tests as workspace packages. |
| pnpm-lock.yaml | Locks new workspace deps (AWS/Azure SDKs, Vitest, etc.). |
| package.json | Updates root formatting/lint scripts (Biome check+format). |
| lefthook.yml | Expands Biome pre-commit glob + runs format after check. |
| docs/changelogs/sdk-typescript.md | Adds TS SDK changelog file for website integration. |
| docs/architecture/adr/0001-sdk-acceptance-test-infrastructure.md | ADR documenting SDK acceptance test infra patterns. |
| ROADMAP.md | Moves TS SDK from “Up Next” to “Shipped” section. |
| .github/workflows/tests-typescript-sdk.yml | New CI workflow to build + test the TS SDK (Vitest). |
| .github/workflows/publish-npm-sdk.yml | New CI workflow to publish TS SDK to npm + tag/release. |
| .github/workflows/coverage-report/coverage-config.json | Adds TS SDK to coverage dashboard config. |
| .github/workflows/coverage-report.yml | Adds TS SDK coverage job + aggregates into dashboard. |
| .github/skills/sdk-release-checklist/SKILL.md | New checklist skill for wiring new SDK releases end-to-end. |
| .github/skills/sdk-acceptance-testing/SKILL.md | New skill documenting acceptance testing patterns for SDKs. |
| .github/skills/doc-sync/SKILL.md | Adds doc-sync guidance for new SDKs/version bumps. |
| .github/copilot-instructions.md | Documents TypeScript SDK architecture and commands. |
| .github/agents/website-designer.agent.md | Updates website agent guidance re SDK wiring/version globals. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| "format": "biome format", | ||
| "format:write": "biome format --write", | ||
| "lint": "secretlint \"**/*\" && biome check --write && tsc --noEmit", | ||
| "format": "biome check --write --unsafe && biome format --write", |
Comment on lines
+15
to
+17
| const LOCALSTACK_IMAGE = 'localstack/localstack:stable'; | ||
| const SECRETS_MAP = path.resolve(__dirname, '../../../../secrets-map.json'); | ||
|
|
Comment on lines
+37
to
+40
| this.container = await new LocalstackContainer(LOCALSTACK_IMAGE) | ||
| .withName('localstack') | ||
| .withEnvironment(envRecord) | ||
| .start(); |
Comment on lines
+24
to
+27
| this.container = await new GenericContainer(LOWKEY_VAULT_IMAGE) | ||
| .withName('lowkey-vault') | ||
| .withExposedPorts(HTTPS_PORT, HTTP_PORT) | ||
| .withEnvironment({ |
Comment on lines
+6
to
+54
| // Mock the AWS and Azure provider modules so we don't need real cloud SDKs | ||
| vi.mock( | ||
| '../../../../src/sdks/typescript/src/infrastructure/aws/aws-ssm-secret-provider.js', | ||
| () => ({ | ||
| AwsSsmSecretProvider: class { | ||
| getSecret = vi.fn(); | ||
| }, | ||
| }), | ||
| ); | ||
|
|
||
| vi.mock( | ||
| '../../../../src/sdks/typescript/src/infrastructure/azure/azure-key-vault-secret-provider.js', | ||
| () => ({ | ||
| AzureKeyVaultSecretProvider: class { | ||
| getSecret = vi.fn(); | ||
| }, | ||
| }), | ||
| ); | ||
|
|
||
| // Mock AWS SDK | ||
| vi.mock('@aws-sdk/client-ssm', () => ({ | ||
| SSMClient: class {}, | ||
| })); | ||
|
|
||
| vi.mock('@aws-sdk/credential-providers', () => ({ | ||
| fromIni: vi.fn().mockReturnValue({}), | ||
| })); | ||
|
|
||
| // Mock Azure SDK | ||
| vi.mock('@azure/identity', () => ({ | ||
| DefaultAzureCredential: class {}, | ||
| })); | ||
|
|
||
| vi.mock('@azure/keyvault-secrets', () => ({ | ||
| SecretClient: class {}, | ||
| })); | ||
|
|
||
| describe('SecretProviderFactory', () => { | ||
| it('Should_CreateAwsProvider_When_NoProviderSpecified', () => { | ||
| // Arrange | ||
| const config = {}; | ||
|
|
||
| // Act | ||
| const actual = createSecretProvider(config); | ||
|
|
||
| // Assert | ||
| expect(actual).toBeDefined(); | ||
| expect(actual.getSecret).toBeDefined(); | ||
| }); |
| ); | ||
|
|
||
| for (const param of response.Parameters ?? []) { | ||
| if (param.Name && param.Value) { |
Comment on lines
+48
to
+51
| function createAwsProvider(profile: string | undefined): AwsSsmSecretProvider { | ||
| const clientOptions = profile ? { credentials: fromIni({ profile }) } : {}; | ||
| const client = new SSMClient(clientOptions); | ||
| return new AwsSsmSecretProvider(client); |
|
|
||
| if (!source.trim()) { | ||
| throw new Error( | ||
| `env_mapping contains an empty file path for environment '${normalized}'.`, |
BREAKING CHANGE: sdk-typescript renamed to sdk-nodejs across all paths, CI workflows, documentation, website i18n, changelogs, and configs. - src/sdks/typescript/ -> src/sdks/nodejs/ - tests/sdks/typescript/ -> tests/sdks/nodejs/ - .github/workflows/tests-typescript-sdk.yml -> tests-nodejs-sdk.yml - docs/changelogs/sdk-typescript.md -> sdk-nodejs.md - Commit scope: sdk-ts -> sdk-node - Git tags: sdk-typescript/vX.Y.Z -> sdk-nodejs/vX.Y.Z - Website i18n: sdkTypescript* -> sdkNodejs* - Logo: sdk-typescript.svg -> sdk-logo.svg
macalbert
changed the title
macalbert
had a problem deploying
to
github-pages
GitHub Actions
Failure
Owner
Author
|
Replaced by new PR with correct branch name (feat/sdk-nodejs). |
Comment on lines
+104
to
+106
| - name: 🚩 Publish to npm | ||
| if: steps.version-check.outputs.version_changed == 'true' | ||
| run: npm publish --provenance --access public |
There was a problem hiding this comment.
🔴 Missing NODE_AUTH_TOKEN environment variable causes npm publish to fail
The npm publish --provenance --access public step at line 106 lacks the NODE_AUTH_TOKEN environment variable needed to authenticate with the npm registry. The actions/setup-node step (line 41-44) with registry-url: "https://registry.npmjs.org" generates an .npmrc containing //registry.npmjs.org/:_authToken=${NODE_AUTH_TOKEN}, but since NODE_AUTH_TOKEN is never set, npm will fail with an authentication error when attempting to publish. Unless the @envilder/sdk npm package is specifically configured for GitHub Actions OIDC trusted publishing (which bypasses the need for a token), this step will fail every time the publish workflow triggers.
Expected pattern per GitHub Actions docs
- name: 🚩 Publish to npm
if: steps.version-check.outputs.version_changed == 'true'
run: npm publish --provenance --access public
env:
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}Note: the existing publish-npm.yml has the same missing env pattern, so this may be intentional if npm trusted publishers (OIDC) is configured for the package.
Suggested change
| - name: 🚩 Publish to npm | |
| if: steps.version-check.outputs.version_changed == 'true' | |
| run: npm publish --provenance --access public | |
| - name: 🚩 Publish to npm | |
| if: steps.version-check.outputs.version_changed == 'true' | |
| run: npm publish --provenance --access public | |
| env: | |
| NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} |
Was this helpful? React with 👍 or 👎 to provide feedback.
macalbert
had a problem deploying
to
github-pages
GitHub Actions
Failure
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
Adds the TypeScript SDK (
@envilder/sdk) — an independent, async-first library for loading secrets from AWS SSM Parameter Store or Azure Key Vault directly into Node.js processes at startup.SDK Architecture
PromisefromMapFile().withProvider().withVaultUrl().inject()load('production', { ... })selects env-specific map filesvalidateSecrets()throws for empty/missing valuesPublic API
Tests
46 unit tests covering all layers:
Website & Docs
#sdk-typescriptsection with install, quick start, resolve, fluent, env routing, and validation examplesdocs/changelogs/sdk-typescript.md)copilot-instructions.mdupdated with TypeScript SDK sectionCommits
feat(sdk-ts): add TypeScript SDK source— domain, application, infrastructure layerstest(sdk-ts): add unit test suite— 46 tests with Vitestdocs(sdk-ts): update website, docs, and roadmap— landing page, docs, i18n, changelogstyle(sdk-ts): apply biome formatting to JSON configs— formatting fixSummary by CodeRabbit
New Features
Documentation