Open
Conversation
Member
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
WalkthroughThis PR adds a new "sdk" CLI subcommand that validates the Enkryptify project configuration, generates an environment-scoped CLI token via HTTP request, spawns a child process with the token injected into environment variables, and updates authentication headers from X-API-Key to Bearer-based Authorization across the HTTP client. Changes
Sequence DiagramsequenceDiagram
participant User as User/CLI
participant SDK as SDK Command
participant Config as Config Loader
participant HTTP as HTTP Client
participant API as Enkryptify API
participant Child as Child Process
User->>SDK: sdk <command> [args]
SDK->>SDK: Validate command exists
SDK->>Config: Load Enkryptify project config
Config-->>SDK: Return project config & workspace
SDK->>HTTP: Create authenticated client
SDK->>API: POST /v1/workspace/{slug}/tokens/cli
API-->>SDK: Return scoped CLI token
SDK->>Child: Spawn child process
Child->>Child: ENKRYPTIFY_TOKEN injected
Note over Child: Execute user command
Child-->>SDK: Exit with code
SDK->>User: Exit with same code
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/cmd/sdk.ts (1)
1-3: Move SDK token issuance behind the provider boundary.This command reaches directly into
@/providers/enkryptify/httpClientand builds an Enkryptify-specific API call itself. That makes the command own provider logic instead of staying at the usual command flow boundary.As per coding guidelines: All commands must follow the structure: validate inputs → get config → get provider → implement logic.
Also applies to: 32-38
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cmd/sdk.ts` around lines 1 - 3, The command is directly importing and using the Enkryptify HTTP client (import http from "@/providers/enkryptify/httpClient") to issue SDK tokens, which leaks provider logic into the command; remove that direct HTTP usage and instead add a provider-level method (e.g., EnkryptifyProvider.issueSdkToken or similar) inside the enkryptify provider module that encapsulates the token issuance, then in src/cmd/sdk.ts (remove the http import) follow the pattern validate inputs → get config → get provider → call provider.issueSdkToken(...) and handle errors via logError/config as before; update any calls around lines 32-38 to use the new provider method so the command no longer builds Enkryptify-specific API calls.
🤖 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/cmd/sdk.ts`:
- Around line 27-39: The code assumes required config fields and the returned
token exist; add explicit validation after loading `setup` and after the HTTP
call: check that `setup.workspace_slug` and `setup.environment_id` are non-empty
before calling `http.post`, and after the post verify `data` and `data.token`
are present and non-empty before assigning to the local `token`; if any check
fails, call `logError` with a descriptive message (including which field is
missing) and exit (as the current flow does) so you never build a request with
`undefined` or continue without credentials (references: `setup`,
`setup.workspace_slug`, `setup.environment_id`, `http.post(...).then(({data}) =>
...)`, and the local `token` variable).
- Around line 52-60: Wrap the Bun.spawn([bin, ...rest], ...) call and the await
proc.exited in a try/catch block so failures to start or wait on the child
process are caught; on error call logError(err) (same helper used earlier in
this file) and exit with a non-zero code, and on success keep the existing
process.exit(exitCode) behavior. Specifically, enclose the Bun.spawn invocation
and the await proc.exited usage (refer to Bun.spawn and proc.exited) in
try/catch, forward the caught error to logError(), and ensure the process exits
appropriately when an exception occurs.
---
Nitpick comments:
In `@src/cmd/sdk.ts`:
- Around line 1-3: The command is directly importing and using the Enkryptify
HTTP client (import http from "@/providers/enkryptify/httpClient") to issue SDK
tokens, which leaks provider logic into the command; remove that direct HTTP
usage and instead add a provider-level method (e.g.,
EnkryptifyProvider.issueSdkToken or similar) inside the enkryptify provider
module that encapsulates the token issuance, then in src/cmd/sdk.ts (remove the
http import) follow the pattern validate inputs → get config → get provider →
call provider.issueSdkToken(...) and handle errors via logError/config as
before; update any calls around lines 32-38 to use the new provider method so
the command no longer builds Enkryptify-specific API calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 255e1c8a-6587-415b-8405-d65dd9bf8998
📒 Files selected for processing (4)
src/cmd/index.tssrc/cmd/sdk.tssrc/providers/enkryptify/auth.tssrc/providers/enkryptify/httpClient.ts
Comment on lines
+27
to
+39
| if (!setup || setup.provider !== "enkryptify") { | ||
| logError("No Enkryptify project configured in this directory. Run `ek configure` first."); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| // 2. Create scoped SDK token (read-only, single environment, 8h) | ||
| let token: string; | ||
| try { | ||
| const { data } = await http.post<{ token: string }>( | ||
| `/v1/workspace/${setup.workspace_slug}/tokens/cli`, | ||
| { environmentId: setup.environment_id }, | ||
| ); | ||
| token = data.token; |
There was a problem hiding this comment.
Validate the loaded config fields and returned token before continuing.
setup.provider === "enkryptify" does not guarantee workspace_slug and environment_id exist, and data.token is used without a runtime check. A stale/broken project config or a malformed 200 response can turn into /v1/workspace/undefined/... requests or a child process launched without credentials.
🛡️ Proposed validation guard
- if (!setup || setup.provider !== "enkryptify") {
+ if (
+ !setup ||
+ setup.provider !== "enkryptify" ||
+ !setup.workspace_slug ||
+ !setup.environment_id
+ ) {
logError("No Enkryptify project configured in this directory. Run `ek configure` first.");
process.exit(1);
}
@@
const { data } = await http.post<{ token: string }>(
`/v1/workspace/${setup.workspace_slug}/tokens/cli`,
{ environmentId: setup.environment_id },
);
+ if (!data?.token) {
+ throw new Error("Failed to create SDK token: response did not include a token.");
+ }
token = data.token;As per coding guidelines: Validate all user inputs, and always throw descriptive errors with context.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cmd/sdk.ts` around lines 27 - 39, The code assumes required config fields
and the returned token exist; add explicit validation after loading `setup` and
after the HTTP call: check that `setup.workspace_slug` and
`setup.environment_id` are non-empty before calling `http.post`, and after the
post verify `data` and `data.token` are present and non-empty before assigning
to the local `token`; if any check fails, call `logError` with a descriptive
message (including which field is missing) and exit (as the current flow does)
so you never build a request with `undefined` or continue without credentials
(references: `setup`, `setup.workspace_slug`, `setup.environment_id`,
`http.post(...).then(({data}) => ...)`, and the local `token` variable).
Comment on lines
+52
to
+60
| const proc = Bun.spawn([bin, ...rest], { | ||
| env: { ...process.env, ENKRYPTIFY_TOKEN: token }, | ||
| stdin: "inherit", | ||
| stdout: "inherit", | ||
| stderr: "inherit", | ||
| }); | ||
|
|
||
| const exitCode = await proc.exited; | ||
| process.exit(exitCode); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/cmd/sdk.tsRepository: Enkryptify/cli
Length of output: 2784
Wrap Bun.spawn() call in try/catch to handle process failures properly.
The code at lines 52-60 lacks error handling for Bun.spawn() and await proc.exited. If the executable cannot be resolved or the wait fails, the error surfaces as an unhandled failure instead of a user-facing CLI error. This violates the pattern used elsewhere in this file (see lines 21-43) where all operations are wrapped with try/catch and errors are passed to logError().
Proposed fix
- const proc = Bun.spawn([bin, ...rest], {
- env: { ...process.env, ENKRYPTIFY_TOKEN: token },
- stdin: "inherit",
- stdout: "inherit",
- stderr: "inherit",
- });
-
- const exitCode = await proc.exited;
- process.exit(exitCode);
+ try {
+ const proc = Bun.spawn([bin, ...rest], {
+ env: { ...process.env, ENKRYPTIFY_TOKEN: token },
+ stdin: "inherit",
+ stdout: "inherit",
+ stderr: "inherit",
+ });
+
+ const exitCode = await proc.exited;
+ process.exit(exitCode);
+ } catch (error: unknown) {
+ logError(error instanceof Error ? error.message : String(error));
+ process.exit(1);
+ }📝 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
| const proc = Bun.spawn([bin, ...rest], { | |
| env: { ...process.env, ENKRYPTIFY_TOKEN: token }, | |
| stdin: "inherit", | |
| stdout: "inherit", | |
| stderr: "inherit", | |
| }); | |
| const exitCode = await proc.exited; | |
| process.exit(exitCode); | |
| try { | |
| const proc = Bun.spawn([bin, ...rest], { | |
| env: { ...process.env, ENKRYPTIFY_TOKEN: token }, | |
| stdin: "inherit", | |
| stdout: "inherit", | |
| stderr: "inherit", | |
| }); | |
| const exitCode = await proc.exited; | |
| process.exit(exitCode); | |
| } catch (error: unknown) { | |
| logError(error instanceof Error ? error.message : String(error)); | |
| process.exit(1); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cmd/sdk.ts` around lines 52 - 60, Wrap the Bun.spawn([bin, ...rest], ...)
call and the await proc.exited in a try/catch block so failures to start or wait
on the child process are caught; on error call logError(err) (same helper used
earlier in this file) and exit with a non-zero code, and on success keep the
existing process.exit(exitCode) behavior. Specifically, enclose the Bun.spawn
invocation and the await proc.exited usage (refer to Bun.spawn and proc.exited)
in try/catch, forward the caught error to logError(), and ensure the process
exits appropriately when an exception occurs.
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
1 participant
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 by CodeRabbit
New Features
Improvements